netdev - RE: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <E4CD12F19ABA0C4D8729E087A761DC3505D84532@ORSMSX101.amr.corp.intel.com>
Date:	Sat, 6 Dec 2014 08:05:19 +0000
From:	"Arad, Ronen" <ronen.arad@...el.com>
To:	Scott Feldman <sfeldma@...il.com>, Netdev <netdev@...r.kernel.org>
CC:	Roopa Prabhu <roopa@...ulusnetworks.com>,
	Jirí Pírko <jiri@...nulli.us>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	Benjamin LaHaise <bcrl@...ck.org>, Thomas Graf <tgraf@...g.ch>,
	john fastabend <john.fastabend@...il.com>,
	"stephen@...workplumber.org" <stephen@...workplumber.org>,
	John Linville <linville@...driver.com>,
	"nhorman@...driver.com" <nhorman@...driver.com>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	"vyasevic@...hat.com" <vyasevic@...hat.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	"buytenh@...tstofly.org" <buytenh@...tstofly.org>,
	Aviad Raveh <aviadr@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>,
	"shm@...ulusnetworks.com" <shm@...ulusnetworks.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>
Subject: RE: [PATCH 2/3] bridge: offload bridge port attributes to switch
 asic if feature flag set



> -----Original Message-----
> From: Scott Feldman [mailto:sfeldma@...il.com]
> Sent: Friday, December 05, 2014 10:29 PM
> To: Arad, Ronen
> Cc: Roopa Prabhu; Netdev; Jirí Pírko; Jamal Hadi Salim; Benjamin LaHaise;
> Thomas Graf; john fastabend; stephen@...workplumber.org; John Linville;
> nhorman@...driver.com; Nicolas Dichtel; vyasevic@...hat.com; Florian
> Fainelli; buytenh@...tstofly.org; Aviad Raveh; David S. Miller;
> shm@...ulusnetworks.com; Andy Gospodarek
> Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic
> if feature flag set
> 
> On Fri, Dec 5, 2014 at 5:04 PM, Arad, Ronen <ronen.arad@...el.com> wrote:
> > I have another case of propagation which is not covered by the proposed
> patch.
> > A recent patch introduced default_pvid attribute for a bridge (so far
> supported only via sysfs and not via netlink).
> > When a port joins a bridge, it inherits a PVID from the default_pvid of the
> bridge.
> > The bridge driver propagates that to the newly created net_bridge_port.
> This is done in br_vlan.c:
> >
> > int nbp_vlan_init(struct net_bridge_port *p) {
> >         int rc = 0;
> >
> >         if (p->br->default_pvid) {
> >                 rc = nbp_vlan_add(p, p->br->default_pvid,
> >                                   BRIDGE_VLAN_INFO_PVID |
> >                                   BRIDGE_VLAN_INFO_UNTAGGED);
> >         }
> >
> >         return rc;
> > }
> >
> > When L2 switching is offloaded to the HW, this PVID setting need to be
> propagated.
> 
> Agreed, it would be nice to have it propagated down, but there is a non-ideal
> work-around.  If you set default_pvid=0 to turn off PVID, then the switch port
> driver can pick some internal VLAN ID just for HW purposes in matching
> untagged pkts.  It's non-ideal because the switch port driver needs to reserve
> a block of VLAN IDs for internal usage or use some other matching
> mechanism to keep untagged pkts within this bridge.

This work-around let the administrator avoid using VID=1 as the default VLAN for untagged frames. However, it does not let the administrator pick a VID of her choice.

> 
> Better to have default_pvid value propagated down.  But, default_pvid is a
> per-bridge property, not a per-bridge-port property.
> RTM_SETLINK/RTM_GETLINK for PF_BRIDGE does have AFSPEC for per-bridge
> and PROTINFO for per-bridge-port, so it seems PVID needs to be part of
> AFSPEC.

I believe AFSPEC is not limited to per-bridge properties. It is per-bridge when the netlink msg's ifindex is that of a bridge and SELF flag is set.
AFSPEC is for a port when the netlink msg's ifindex is that of an enslaved port device and MASTER flag is set (or neither MASTER nor SELF flag is set)
PVID is one of the flags associated with a VID in bridge_vlan_info.
default_pvid is not currently supported by netlink. A new IFLA_BRIDGE_DEFAULT_PVID could be introduced to carry this property when a nlmsg is directed at a bridge.

> 
> >However, it does not come via ndo_bridge_setlink. The proposed
> propagation at br_setlink or an up level one at rtnetlink are not capable of
> handling this case.
> > One possible way for handling that is to replace the call to
> >nbp_vlan_add with a call to a new function let's say  int
> >br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
> This function will compose a netlink message with VLAN filtering information
> (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload
> support proposed by Roopa.
> >
> > If this is an acceptable course of action, I could work on such patch.
> >
> >
> >> -----Original Message-----
> >> From: netdev-owner@...r.kernel.org [mailto:netdev-
> >> owner@...r.kernel.org] On Behalf Of Arad, Ronen
> >> Sent: Friday, December 05, 2014 3:21 PM
> >> To: Roopa Prabhu; Scott Feldman; Netdev
> >> Cc: Jirí Pírko; Jamal Hadi Salim; Benjamin LaHaise; Thomas Graf; john
> >> fastabend; stephen@...workplumber.org; John Linville;
> >> nhorman@...driver.com; Nicolas Dichtel; vyasevic@...hat.com; Florian
> >> Fainelli; buytenh@...tstofly.org; Aviad Raveh; David S. Miller;
> >> shm@...ulusnetworks.com; Andy Gospodarek
> >> Subject: RE: [PATCH 2/3] bridge: offload bridge port attributes to
> >> switch asic if feature flag set
> >>
> >>
> >>
<snip>