From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [RFC patch net-next-2.6] net: convert bonding to use rx_handler Date: Thu, 17 Feb 2011 14:49:18 +0100 Message-ID: <20110217134918.GA11679@psychotron.redhat.com> References: <20110217125221.GA10436@psychotron.redhat.com> <1297948835.2604.50.camel@edumazet-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: netdev@vger.kernel.org, davem@davemloft.net, shemminger@linux-foundation.org, kaber@trash.net, fubar@us.ibm.com, nicolas.2p.debian@gmail.com, andy@greyhouse.net To: Eric Dumazet Return-path: Received: from mx1.redhat.com ([209.132.183.28]:29728 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756373Ab1BQNti (ORCPT ); Thu, 17 Feb 2011 08:49:38 -0500 Content-Disposition: inline In-Reply-To: <1297948835.2604.50.camel@edumazet-laptop> Sender: netdev-owner@vger.kernel.org List-ID: Thu, Feb 17, 2011 at 02:20:35PM CET, eric.dumazet@gmail.com wrote: >Le jeudi 17 f=E9vrier 2011 =E0 13:52 +0100, Jiri Pirko a =E9crit : >> Hello. >>=20 >> This is an attempt to convert bonding to use rx_handler. Result shou= ld be >> cleaner __netif_receive_skb() with much less exceptions needed. I th= ink I >> covered all aspects, not sure though. I gave this quick smoke test o= n my >> testing env. Please comment, test. >>=20 >> Thanks! >>=20 >> Signed-off-by: Jiri Pirko >> --- >> drivers/net/bonding/bond_main.c | 75 ++++++++++++++++++++- >> include/linux/skbuff.h | 1 + >> net/core/dev.c | 144 +++++++++++-----------------= ---------- >> 3 files changed, 117 insertions(+), 103 deletions(-) >>=20 >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/b= ond_main.c > > >> + >> +static struct sk_buff *bond_handle_frame(struct sk_buff *skb) >> +{ >> + struct net_device *slave_dev; >> + struct net_device *bond_dev; >> + >> + skb =3D skb_share_check(skb, GFP_ATOMIC); >> + if (unlikely(!skb)) >> + return NULL; >> + slave_dev =3D skb->dev; >> + bond_dev =3D slave_dev->master; > >I suggest being 10%% safe here : > > bond_dev =3D ACCESS_ONCE(slave_dev->master); Right, will change this. > >> + if (unlikely(!bond_dev)) >> + return skb; >> + >> + if (bond_dev->priv_flags & IFF_MASTER_ARPMON) >> + slave_dev->last_rx =3D jiffies; >> + >> + if (bond_should_deliver_exact_match(skb, slave_dev, bond_dev)) { >> + skb->deliver_no_wcard =3D 1; >> + return skb; >> + } >> + >> + skb->dev =3D bond_dev; >> + >> + if (bond_dev->priv_flags & IFF_MASTER_ALB && >> + bond_dev->priv_flags & IFF_BRIDGE_PORT && >> + skb->pkt_type =3D=3D PACKET_HOST) { >> + u16 *dest =3D (u16 *) eth_hdr(skb)->h_dest; >> + >> + memcpy(dest, bond_dev->dev_addr, ETH_ALEN); >> + } >> + >> + netif_rx(skb); >> + return NULL; >> +} >> + > > >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h >> index 31f02d0..15b54ea 100644 >> --- a/include/linux/skbuff.h >> +++ b/include/linux/skbuff.h >> @@ -325,6 +325,7 @@ struct sk_buff { >> =20 >> struct sock *sk; >> struct net_device *dev; >> + struct net_device *orig_dev; >> =20 >> /* >> * This is the control buffer. It is free to use for every > >Thats a problem. lifetime of this field is so small, I wonder if you >cant find a solution to handle this differently. Maybe a percpu >variable, or in cb[] ? Yes, I was not feeling absolutely comfortable puting this here. You mean global percpu variable living in net/core/dev.c? I must say I would probably like skb->orig_dev more than that. As for cb - I do not like that much. Also I think there might be collision e.g. with bridge code. > > >