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

[Roopa Prabhu <roopa@cumulusnetworks.com>]

On 12/6/14, 12:05 AM, Arad, Ronen wrote:
>
>> -----Original Message-----
>> From: Scott Feldman [mailto:sfeldma@gmail.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@networkplumber.org; John Linville;
>> nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com; Florian
>> Fainelli; buytenh@wantstofly.org; Aviad Raveh; David S. Miller;
>> shm@cumulusnetworks.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@intel.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.
correct.
> 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.
>
>
correct again. And yes,  a netlink attribute to set default pvid is due.