[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8628FE4E7912BF47A96AE7DD7BAC0AADDDE6470191@SJEXCHCCR02.corp.ad.broadcom.com>
Date: Thu, 8 Jul 2010 05:58:42 -0700
From: "Vladislav Zolotarov" <vladz@...adcom.com>
To: "Vladislav Zolotarov" <vladz@...adcom.com>,
"Pedro Garcia" <pedro.netdev@...devamos.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
cc: "Patrick McHardy" <kaber@...sh.net>,
"Ben Hutchings" <bhutchings@...arflare.com>,
"Eric Dumazet" <eric.dumazet@...il.com>
Subject: RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
(802.1p packet)
Opps...
Sorry. Wrong thread. :)
Pls., discard!
> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
> Behalf Of Vladislav Zolotarov
> Sent: Thursday, July 08, 2010 3:55 PM
> To: Pedro Garcia; netdev@...r.kernel.org
> Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> Subject: RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> (802.1p packet)
>
> Margaret,
> In order to keep exploring this we need the following data:
> 1. What is the scenario exactly? What is vMotion? Which kind of operation
> does it do? Which kind of traffic does it pass?
> 2. What is the nature of the failure- driver hang? PSOD? Loss of traffic?
> 3. We need a grcdump after the failure.
>
> Thanks,
> vlad
>
> > -----Original Message-----
> > From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
> > Behalf Of Pedro Garcia
> > Sent: Wednesday, June 16, 2010 11:49 AM
> > To: netdev@...r.kernel.org
> > Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
> > Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> > (802.1p packet)
> >
> > On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet <eric.dumazet@...il.com>
> > wrote:
> > > Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit :
> > >> Ben Hutchings wrote:
> > >> > On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote:
> > >> >
> > >> >> On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings
> > >> >> <bhutchings@...arflare.com> wrote:
> > >> >>
> > >> >>> I have no particular opinion on this change, but you need to read
> and
> > >> >>> follow Documentation/SubmittingPatches.
> > >> >>>
> > >> >>> Ben.
> > >> >>>
> > >> >> Sorry, first kernel patch, and I did not know about it. I resubmit
> > >> >> with
> > >> >> the correct style / format:
> > >> >>
> > >> > [...]
> > >> >
> > >> > Sorry, no you haven't.
> > >> >
> > >> > - Networking changes go through David Miller's net-next-2.6 tree so
> you
> > >> > need to use that as the baseline, not 2.6.26
> > >> > - Patches should be applicable with -p1, not -p0 (so if you use diff,
> > >> > you should run it from one directory level up)
> > >> > - The patch was word-wrapped
> > >>
> > >> Additionally:
> > >>
> > >> - please use the proper comment style, meaning each line begins
> > >> with a '*'
> > >>
> > >> - the pr_debug statements may be useful for debugging, but are
> > >> a bit excessive for the final version
> > >>
> > >> - + /* 2010-06-13: Pedro Garcia
> > >>
> > >> We have changelogs for this, simply explaining what the code
> > >> does is enough.
> > >>
> > >> - Please CC the maintainer (which is me)
> > >> --
> > >
> > > Pedro, we have two kind of vlan setups :
> > >
> > > accelerated and non accelerated ones.
> > >
> > > Your patch address non accelated ones only, since you only touch
> > > vlan_skb_recv()
> > >
> > > Accelerated vlan can follow these paths :
> > >
> > > 1) NAPI devices
> > >
> > > vlan_gro_receive() -> vlan_gro_common()
> > >
> > > 2) non NAPI devices
> > >
> > > __vlan_hwaccel_rx()
> > >
> > > So you might also patch __vlan_hwaccel_rx() and vlan_gro_common()
> > >
> > > Please merge following bits to your patch submission :
> > >
> > > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868
> > >
> > >
> > > Good luck for your first patch !
> >
> > Here it is again. I added the modifications in
> > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW
> > accelerated incoming packets (it did not apply cleanly on the last version
> of
> > the kernel, so I applied manually). Now, if the VLAN 0 is not explicitly
> > created by the user, VLAN 0 packets will be treated as no VLAN (802.1p
> > packets), instead of dropping them.
> >
> > The patch is now for two files: vlan_core (accel) and vlan_dev (non accel)
> >
> > I can not test on HW accelerated devices, so if someone can check it I will
> > appreciate (even though in the thread above it looked like yes). For non
> > accel I tessted in 2.6.26. Now the patch is for
> > net-next-2.6, and it compiles OK, but I a have to setup a test environment
> to
> > check it is still OK (should, but better to test).
> >
> > Signed-off-by: Pedro Garcia <pedro.netdev@...devamos.com>
> > --
> > diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> > index 50f58f5..daaca31 100644
> > --- a/net/8021q/vlan_core.c
> > +++ b/net/8021q/vlan_core.c
> > @@ -8,6 +8,9 @@
> > int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
> > u16 vlan_tci, int polling)
> > {
> > + struct net_device *vlan_dev;
> > + u16 vlan_id;
> > +
> > if (netpoll_rx(skb))
> > return NET_RX_DROP;
> >
> > @@ -16,10 +19,14 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct
> > vlan_group *grp,
> >
> > skb->skb_iif = skb->dev->ifindex;
> > __vlan_hwaccel_put_tag(skb, vlan_tci);
> > - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> > + vlan_id = vlan_tci & VLAN_VID_MASK;
> > + vlan_dev = vlan_group_get_device(grp, vlan_id);
> >
> > - if (!skb->dev)
> > - goto drop;
> > + if (vlan_dev)
> > + skb->dev = vlan_dev;
> > + else
> > + if (vlan_id)
> > + goto drop;
> >
> > return (polling ? netif_receive_skb(skb) : netif_rx(skb));
> >
> > @@ -82,16 +89,22 @@ vlan_gro_common(struct napi_struct *napi, struct
> > vlan_group *grp,
> > unsigned int vlan_tci, struct sk_buff *skb)
> > {
> > struct sk_buff *p;
> > + struct net_device *vlan_dev;
> > + u16 vlan_id;
> >
> > if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master)))
> > skb->deliver_no_wcard = 1;
> >
> > skb->skb_iif = skb->dev->ifindex;
> > __vlan_hwaccel_put_tag(skb, vlan_tci);
> > - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK);
> > -
> > - if (!skb->dev)
> > - goto drop;
> > + vlan_id = vlan_tci & VLAN_VID_MASK;
> > + vlan_dev = vlan_group_get_device(grp, vlan_id);
> > +
> > + if (vlan_dev)
> > + skb->dev = vlan_dev;
> > + else
> > + if (vlan_id)
> > + goto drop;
> >
> > for (p = napi->gro_list; p; p = p->next) {
> > NAPI_GRO_CB(p)->same_flow =
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index 5298426..65512c3 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -142,6 +142,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct
> net_device
> > *dev,
> > {
> > struct vlan_hdr *vhdr;
> > struct vlan_rx_stats *rx_stats;
> > + struct net_device *vlan_dev;
> > u16 vlan_id;
> > u16 vlan_tci;
> >
> > @@ -157,8 +158,18 @@ int vlan_skb_recv(struct sk_buff *skb, struct
> net_device
> > *dev,
> > vlan_id = vlan_tci & VLAN_VID_MASK;
> >
> > rcu_read_lock();
> > - skb->dev = __find_vlan_dev(dev, vlan_id);
> > - if (!skb->dev) {
> > + vlan_dev = __find_vlan_dev(dev, vlan_id);
> > +
> > + /* If the VLAN device is defined, we use it.
> > + * If not, and the VID is 0, it is a 802.1p packet (not
> > + * really a VLAN), so we will just netif_rx it later to the
> > + * original interface, but with the skb->proto set to the
> > + * wrapped proto: we do nothing here.
> > + */
> > +
> > + if (vlan_dev) {
> > + skb->dev = vlan_dev;
> > + } else if (vlan_id) {
> > pr_debug("%s: ERROR: No net_device for VID: %u on dev:
> %s\n",
> > __func__, vlan_id, dev->name);
> > goto err_unlock;
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> N�����r��y���b�X��ǧv�^�){.n�+���z�^�)���w*
> jg���.�����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a����.�G���h�.�j:+v���w�٥
Powered by blists - more mailing lists