on Mon, 13 Dec 2010 06:48:06AM +0900 Simon Horman (horms@xxxxxxxxxxxx) wrote:
> [ CCed Hans Schillstrom and Julian Anastasov ]
>
> On Sun, Dec 12, 2010 at 07:42:29PM +0800, Tinggong Wang wrote:
> > Signed-off-by: Tinggong Wang <wangtinggong@xxxxxxxxx>
> > ---
> > net/netfilter/ipvs/ip_vs_sync.c | 13 ++++++++-----
> > 1 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 7632a17..2b6b0cb 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -315,11 +315,6 @@ static void ip_vs_process_message(const char *buffer, const size_t buflen)
> > char *p;
> > int i;
> >
> > - if (buflen < SYNC_MESG_HEADER_LEN) {
> > - IP_VS_ERR_RL("sync message header too short\n");
> > - return;
> > - }
> > -
> > /* Convert size back to host byte order */
> > m->size = ntohs(m->size);
> >
> > @@ -823,6 +818,14 @@ static int sync_thread_backup(void *data)
> > break;
> > }
> >
> > + /* throw invalid data before local_bh_disable,
> > + * so performance won't be downgraded by it
> > + */
> > + if (len < SYNC_MESG_HEADER_LEN) {
> > + IP_VS_ERR_RL("sync message header too short\n");
> > + continue;
> > + }
> > +
> > /* disable bottom half, because it accesses the data
> > shared by softirq while getting/creating conns */
> > local_bh_disable();
> > --
> > 1.7.2.3
> >
>
> Could you explain the motivation for this change?
in my opinion, before local_bh_disable, should ensure packets are look
like more resonable.
local_bh_disable will disable all bottom-half processing on local cpu,
if the multicast group flood of packets containing bad sync message,
local cpu will be busy doing local_bh_disable and local_bh_enable.
if the backup pc has only one cpu, all other tasks will be pending until
the flood finished.
--
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
