LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [patch] ipvs: off by one in set_sctp_state()

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [patch] ipvs: off by one in set_sctp_state()
Cc: Simon Horman <horms@xxxxxxxxxxxx>, Dan Carpenter <dan.carpenter@xxxxxxxxxx>, Wensong Zhang <wensong@xxxxxxxxxxxx>, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>, Patrick McHardy <kaber@xxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, netdev@xxxxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, netfilter@xxxxxxxxxxxxxxx, coreteam@xxxxxxxxxxxxx
From: Moahn Reddy <mohanreddykv@xxxxxxxxx>
Date: Mon, 22 Apr 2013 14:47:10 +0530
Hi,

I see there is no problem in the code regarding the state change. And the thing why I took 255 in the sctp_events array is that as per the sctp specification, the 255 message is reserved, so I thought 0 to 254 messages are enough.

Do you see any problem with the ipvs sctp code in the field?. Please let me know if you see any such, I will try to fix.

Thanks,
Mohan

On Monday 22 April 2013 11:33 AM, Julian Anastasov wrote:
        Hello,

On Mon, 22 Apr 2013, Simon Horman wrote:

        There are more confusing (still, non-fatal)
problems in this IPVS-SCTP support, eg.

         if (direction == IP_VS_DIR_OUTPUT)
-               event++;
+               event *= 2;

        I guess we are running with wrong timeouts.
IMHO there seem to be many problems with SCTP, but it is good to
fix the ones we find as we find them.
        At the time I found it (during IPVS optimizations
development), it didn't looked fatal, I preferred to
allocate more time for SCTP for debugging.

Would you like to make a patch for the above change or should I?
        May be the code is correct, my mistake. I was
confused from the order in sctp_events[] but ipvs_sctp_event_t
allocates values for _SER states.

        Also, I'm not sure we support properly the
one-way states as done for TCP (IP_VS_DIR_INPUT_ONLY).
May be this code deserves more serious review, for example,
net/netfilter/nf_conntrack_proto_sctp.c looks as good
source for comparison.
I believe it does need a more serious review.
Regards

--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>