| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8628FE4E7912BF47A96AE7DD7BAC0AADDDC6675C68@SJEXCHCCR02.corp.ad.broadcom.com>
Date: Mon, 24 May 2010 00:44:10 -0700
From: "Vladislav Zolotarov" <vladz@...adcom.com>
To: "Eric Dumazet" <eric.dumazet@...il.com>
cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: Receiving of priority tagged packets
Great! Thanks. I'll check it shortly.
However this patch handles the problem only in the HW accelerated path, which is used only when the NIC supports the HW acceleration of RX VLAN parsing and (the most important) at least one VLAN is configured for the current device (this will initialize the dev->vlgrp, otherwise it's NULL and u r not allowed to call vlan_hwaccel_rx()). So if I haven’t configured any VLAN this patch won't help. Namely we need to fix netif_receive_skb(skb) as well.
I would also like to ask u another question: formally VID=0 is not a valid VID (according to the same spec).
However there is this ethX.0 semantics which is a bit confusing. So, don't u think that together with the patch below and the addition described above we should also forbid the configuration of VID=0 (vconfig add ethX 0)?
Thanks a lot again.
Vlad
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@...il.com]
> Sent: Sunday, May 23, 2010 11:33 PM
> To: Vladislav Zolotarov
> Cc: netdev@...r.kernel.org
> Subject: RE: Receiving of priority tagged packets
>
> Le dimanche 23 mai 2010 à 03:51 -0700, Vladislav Zolotarov a écrit :
> >
> > > -----Original Message-----
> > > From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On
> > > Behalf Of Vladislav Zolotarov
> > > Sent: Sunday, May 23, 2010 1:23 PM
> > > To: Eric Dumazet
> > > Cc: netdev@...r.kernel.org
> > > Subject: RE: Receiving of priority tagged packets
> > >
> > >
> > > > -----Original Message-----
> > > > From: Eric Dumazet [mailto:eric.dumazet@...il.com]
> > > > Sent: Sunday, May 23, 2010 1:07 PM
> > > > To: Vladislav Zolotarov
> > > > Cc: netdev@...r.kernel.org
> > > > Subject: Re: Receiving of priority tagged packets
> > > >
> > > > Le dimanche 23 mai 2010 à 02:36 -0700, Vladislav Zolotarov a écrit :
> > > > > Hello,
> > > > > We were playing with FCoE in our labs and saw the strange behavior of
> > > Linux
> > > > networking stack in regard to priority-tagged frames (the ones that
> have a
> > > > zero VID in a VLAN tag). We saw that until we explicitly added a zero
> vlan
> > > > interface (vconfig add ethX 0) the stack refused to accept such packets
> > > both
> > > > in HW VLAN acceleration mode (skb is indicated using
> > > > vlan_hwaccel_receive_skb()) and in a regular mode (skb is indicated
> with
> > > > netif_receive_skb()).
> > > > >
> > > > > However "IEEE Std 802.1Q, 2006 Edition DRAFT D0.1" in section 6.7
> states
> > > > the following:
> > > > >
> > > > > Each Bridge Port shall support the following parameters for use by
> these
> > > > (EISS tagging and detagging) functions:
> > > > > c) an Acceptable Frame Types parameter with at least one of the
> > > > following values:
> > > > > 1) Admit Only VLAN-tagged frames;
> > > > > 2) Admit Only Untagged and Priority-tagged frames;
> > > > > 3) Admit All frames
> > > > >
> > > > > So I guess this means that priority tagged frames should be accepted
> > > > together with the untagged frames on the default interface ethX.
> > > > >
> > > > > Could anyone explain, pls., what's the expected behavior of the Linux
> > > > Networking Stack in regard to the priority-tagged frames and what's
> > > expected
> > > > to be configured in order to start accepting them?
> > > > >
> > > > > Thanks in advance,
> > > > > vlad
> > > >
> > > > So if eth0.0 is setup, incoming vlanid=0 frames are delivered to
> eth0.0,
> > > > OK ? (This works in and out since commit 05423b241311c93)
> > > >
> > > > Now, if eth0.0 is not setup, you believe these frames should be
> directed
> > > > to eth0, as if they were not tagged ?
> > > >
> > > > That seems a bit strange.
> > >
> > > Well, as far as I understood this is what IEEE 802.1Q spec states...
> >
> > From the same spec:
> >
> > ***************
> > 3.38 Priority-tagged frame: A tagged frame whose tag header carries
> priority information but carries no
> > VLAN identification information.
> > ***************
> >
> > Namely priority-tagged frames are frames that carry no VLAN identification
> information and namely should be treated as non-VLAN packets.
> >
> > However, as u mentioned above, current Networking Stack implementation
> handles them as if they have VID 0.
> >
> > These two are quite different ways to handle the frame, don’t u think?
>
> Following patch should fix it ?
>
> Fallback to ethX is ethX.0 is not used.
>
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index bd537fc..909f6e7 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)))
> goto drop;
>
> 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 =
>
>
>
>
Powered by blists - more mailing lists