netdev - RE: [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge

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]

Date:	Tue, 31 Mar 2015 00:08:34 +0000
From:	"Arad, Ronen" <ronen.arad@...el.com>
To:	Scott Feldman <sfeldma@...il.com>, Netdev <netdev@...r.kernel.org>
CC:	roopa <roopa@...ulusnetworks.com>,
	Jirí Pírko <jiri@...nulli.us>,
	Guenter Roeck <linux@...ck-us.net>,
	Florian Fainelli <f.fainelli@...il.com>
Subject: RE: [PATCH net-next 11/18] switchdev: remove old
 netdev_switch_port_bridge_setlink



>-----Original Message-----
>From: Scott Feldman [mailto:sfeldma@...il.com]
>Sent: Monday, March 30, 2015 2:28 PM
>To: Arad, Ronen
>Cc: roopa; Netdev; Jirí Pírko; Guenter Roeck; Florian Fainelli
>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>netdev_switch_port_bridge_setlink
>
>On Mon, Mar 30, 2015 at 1:46 PM, Arad, Ronen <ronen.arad@...el.com> wrote:
>>
>>
>>>-----Original Message-----
>>>From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
>>>Behalf Of Scott Feldman
>>>Sent: Monday, March 30, 2015 1:20 PM
>>>To: roopa
>>>Cc: Netdev; Jiří Pírko; Guenter Roeck; Florian Fainelli
>>>Subject: Re: [PATCH net-next 11/18] switchdev: remove old
>>>netdev_switch_port_bridge_setlink
>>>
>>>On Mon, Mar 30, 2015 at 6:23 AM, roopa <roopa@...ulusnetworks.com> wrote:
>>>> On 3/30/15, 1:40 AM, sfeldma@...il.com wrote:
>>>>>
>>>>> From: Scott Feldman <sfeldma@...il.com>
>>>>>
>>>>> New attr-based bridge_setlink can recurse lower devs and recover on err,
>>>>> so
>>>>> remove old wrapper.  Also, restore br_setlink back to original and don't
>>>>> call
>>>>> into SELF port driver.  rtnetlink.c:bridge_setlink already does a call
>>>>> into
>>>>> port driver for SELF.
>>>>>
>>>>> Signed-off-by: Scott Feldman <sfeldma@...il.com>
>>>>
>>>> removing this now requires every vlan add to be a two step process, why ?
>>>
>>>No, that's not true.  You want to use
>>>ndo_vlan_rx_add_vid/ndo_vlan_rx_kill_vid in your port driver, and then
>>>using either vlan driver standalone or the bridge driver vlan support
>>>will work.
>>>
>>>Try it.  Implement ndo_vlan_rx_add_vid in your port driver and verify
>>>you get called to add VLAN to port with either:
>>>
>>>    bridge vlan add dev swp1 vid 10
>>>
>>>    -or-
>>>
>>>    vconfig add swp1 10
>>>
>>>Same for deleting a VLAN, either of these two commands call into the
>>>port driver ndo_vlan_rx_kill_vid:
>>>
>>>    bridge vlan del dev swp1 vid 10
>>>
>>>    -or-
>>>
>>>    vconfig rem swp1 10
>>>
>>>
>>>> bridge vlan add dev swp1 vid 10
>>>> bridge vlan add dev swp1 vid 10 self
>>>
>>>Not necessary.  The first command is sufficient if using
>>>ndo_vlan_rx_add_vid.
>>
>> This is not sufficient for VLAN filtering. Ndo_vlan_rx_add_vid does not
>> provide the vinfo flags PVID and UNTAGGED. Therefore it is not
>> an adequate replacement for propagating setlink/dellink messages to the
>> swithport driver or an alternative via swdev_attr.
>
>Glad you bring that point up.  I think these can get cast as port
>attrs and set using swdev_attr.  This is something swdev attr should
>open up is allowing more settings to be pushed down to port driver.
>I'll look into this one and include it with v2.

It could be beneficial to build extensibility into swdev_attr.
An experimenter attribute designed to carry arbitrary data could allow
for passing new attributes and implementation specific attributes
without affecting any existing switchdev driver:

enum swdev_attr_id {
 	SWDEV_ATTR_UNDEFINED,
	SWDEV_ATTR_EXPERIMENTER,
 	SWDEV_ATTR_PORT_PARENT_ID,
	SWDEV_ATTR_PORT_STP_STATE,
 };


struct swdev_experimenter_attr {
	u32 exp_id; 	/* - MSB 0: low-order bytes are IEEE OUI */
			/* - MSB != 0: reserved for netdev */
	u32 exp_attr;
	u16 exp_attr_size;
	const void *exp_attr_data;
}

struct swdev_attr {
	enum swdev_attr_id attr;
	u32 flags;

 	union {
 		struct netdev_phys_item_id ppid;	/* PORT_PARENT_ID */
 		u8 stp_state;				/* PORT_STP_STATE */
		unsigned long brport_flags;	/* PORT_BRIDGE_FLAGS */
		/* netdev defined attributes abobe this line */
		struct swdev_experimenter_attr exp_attr;
 	};