| 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: <CAE4R7bCfi-90pccDJN350bdeQi2R4H5Xbu3-BADZ29vWhfkJiA@mail.gmail.com>
Date: Fri, 28 Nov 2014 02:51:08 -0800
From: Scott Feldman <sfeldma@...il.com>
To: Roopa Prabhu <roopa@...ulusnetworks.com>
Cc: Jiri Pirko <jiri@...nulli.us>, Netdev <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>,
"nhorman@...driver.com" <nhorman@...driver.com>,
Andy Gospodarek <andy@...yhouse.net>,
Thomas Graf <tgraf@...g.ch>,
"dborkman@...hat.com" <dborkman@...hat.com>,
"ogerlitz@...lanox.com" <ogerlitz@...lanox.com>,
"jesse@...ira.com" <jesse@...ira.com>,
"pshelar@...ira.com" <pshelar@...ira.com>,
"azhou@...ira.com" <azhou@...ira.com>,
"ben@...adent.org.uk" <ben@...adent.org.uk>,
"stephen@...workplumber.org" <stephen@...workplumber.org>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
"vyasevic@...hat.com" <vyasevic@...hat.com>,
Cong Wang <xiyou.wangcong@...il.com>,
"Fastabend, John R" <john.r.fastabend@...el.com>,
Eric Dumazet <edumazet@...gle.com>,
Jamal Hadi Salim <jhs@...atatu.com>,
Florian Fainelli <f.fainelli@...il.com>,
John Linville <linville@...driver.com>,
"jasowang@...hat.com" <jasowang@...hat.com>,
"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
Nicolas Dichtel <nicolas.dichtel@...nd.com>,
"ryazanov.s.a@...il.com" <ryazanov.s.a@...il.com>,
"buytenh@...tstofly.org" <buytenh@...tstofly.org>,
Aviad Raveh <aviadr@...lanox.com>,
"nbd@...nwrt.org" <nbd@...nwrt.org>,
Alexei Starovoitov <alexei.starovoitov@...il.com>,
Neil Jerram <Neil.Jerram@...aswitch.com>,
"ronye@...lanox.com" <ronye@...lanox.com>,
"simon.horman@...ronome.com" <simon.horman@...ronome.com>,
"alexander.h.duyck@...hat.com" <alexander.h.duyck@...hat.com>,
"Ronciak, John" <john.ronciak@...el.com>,
"mleitner@...hat.com" <mleitner@...hat.com>,
Shrijeet Mukherjee <shrijeet@...il.com>,
Andy Gospodarek <gospo@...ulusnetworks.com>,
Benjamin LaHaise <bcrl@...ck.org>
Subject: Re: [patch net-next v3 08/17] bridge: call netdev_sw_port_stp_update
when bridge port STP status changes
On Fri, Nov 28, 2014 at 2:05 AM, Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
> On 11/25/14, 5:35 PM, Scott Feldman wrote:
>>
>> The
>> bridge driver or external STP process (msptd) is still controlling STP
>> state for the port and processing the BPDUs. When the state changes
>> on the port, the bridge driver is letting HW know, that's it.
>
>
> I understand that. In which case, we should not call it stp state.
> It is just port state.
Sure, call it port state but it takes on BR_STATE_xxx values which
just so happen to correspond exactly to STP states.
> And since it is yet another port attribute like port
> priority,
> we should be able to use the same api to offload it to hw just like the
> other port attributes.
Well it does...see ndo_bridge_setlink in bridge driver, br_setport
where IFLA_BRPORT_STATE is handled...it calls br_set_port_state(),
which calls into the swdev port driver. That's for the case where
user or external processing is setting STP state. For the case where
the bridge itself is managing the STP state, the bridge will make the
same br_set_port_state() call to adjust the port state.
> And, Thats why i tried to generalize all bridge port attribute set by
> introducing one generic netdev_sw_port_set_attr api.
> https://marc.info/?l=linux-netdev&m=141661018619712&w=2
>
>
> And coming back to my original comment in this thread,
> the port state should be offloaded to hw only when the swdev mode (or hw
> offload mode ;) is set.
>
>
>> If the
>> port driver can't do anything with that notification, then it should
>> not implement ndo_switch_port_stp_update. If it does implement
>> ndo_switch_port_stp_update, then it can adjust its HW (e.g. disable
>> port if BR_DISABLED, etc), and return err code if somehow it failed
>> while adjusting HW.
>>
>> This is not offloading STP state ctrl plane to HW. The ctrl plane is
>> kept in bridge driver (or mstpd) in SW. HW stays dumb in this model.
>> The bridge currently has policy control to turn on/off STP per bridge
>> and a netlink hook for external processes to change STP state.
>>
>> On Tue, Nov 25, 2014 at 12:48 PM, Roopa Prabhu
>> <roopa@...ulusnetworks.com> wrote:
>>>
>>> On 11/25/14, 2:28 AM, Jiri Pirko wrote:
>>>>
>>>> From: Scott Feldman <sfeldma@...il.com>
>>>>
>>>> To notify switch driver of change in STP state of bridge port, add new
>>>> .ndo op and provide switchdev wrapper func to call ndo op. Use it in
>>>> bridge
>>>> code then.
>>>>
>>>> Signed-off-by: Scott Feldman <sfeldma@...il.com>
>>>> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
>>>> ---
>>>> v2->v3:
>>>> -changed "sw" string to "switch" to avoid confusion
>>>> v1->v2:
>>>> -no change
>>>> ---
>>>> include/linux/netdevice.h | 5 +++++
>>>> include/net/switchdev.h | 7 +++++++
>>>> net/bridge/br_stp.c | 2 ++
>>>> net/switchdev/switchdev.c | 19 +++++++++++++++++++
>>>> 4 files changed, 33 insertions(+)
>>>>
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index ce096dc..66cb64e 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -1024,6 +1024,9 @@ typedef u16 (*select_queue_fallback_t)(struct
>>>> net_device *dev,
>>>> * Called to get an ID of the switch chip this port is part of.
>>>> * If driver implements this, it indicates that it represents a
>>>> port
>>>> * of a switch chip.
>>>> + * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state);
>>>> + * Called to notify switch device port of bridge port STP
>>>> + * state change.
>>>> */
>>>> struct net_device_ops {
>>>> int (*ndo_init)(struct net_device *dev);
>>>> @@ -1180,6 +1183,8 @@ struct net_device_ops {
>>>> #ifdef CONFIG_NET_SWITCHDEV
>>>> int (*ndo_switch_parent_id_get)(struct
>>>> net_device *dev,
>>>> struct
>>>> netdev_phys_item_id *psid);
>>>> + int (*ndo_switch_port_stp_update)(struct
>>>> net_device *dev,
>>>> + u8 state);
>>>> #endif
>>>> };
>>>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>> index 7a52360..8a6d164 100644
>>>> --- a/include/net/switchdev.h
>>>> +++ b/include/net/switchdev.h
>>>> @@ -16,6 +16,7 @@
>>>> int netdev_switch_parent_id_get(struct net_device *dev,
>>>> struct netdev_phys_item_id *psid);
>>>> +int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>>> #else
>>>> @@ -25,6 +26,12 @@ static inline int
>>>> netdev_switch_parent_id_get(struct
>>>> net_device *dev,
>>>> return -EOPNOTSUPP;
>>>> }
>>>> +static inline int netdev_switch_port_stp_update(struct net_device
>>>> *dev,
>>>> + u8 state)
>>>> +{
>>>> + return -EOPNOTSUPP;
>>>> +}
>>>> +
>>>> #endif
>>>> #endif /* _LINUX_SWITCHDEV_H_ */
>>>> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
>>>> index 2b047bc..35e016c 100644
>>>> --- a/net/bridge/br_stp.c
>>>> +++ b/net/bridge/br_stp.c
>>>> @@ -12,6 +12,7 @@
>>>> */
>>>> #include <linux/kernel.h>
>>>> #include <linux/rculist.h>
>>>> +#include <net/switchdev.h>
>>>> #include "br_private.h"
>>>> #include "br_private_stp.h"
>>>> @@ -39,6 +40,7 @@ void br_log_state(const struct net_bridge_port *p)
>>>> void br_set_state(struct net_bridge_port *p, unsigned int state)
>>>> {
>>>> p->state = state;
>>>> + netdev_switch_port_stp_update(p->dev, state);
>>>> }
>>>> /* called under bridge lock */
>>>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>>> index 66973de..d162b21 100644
>>>> --- a/net/switchdev/switchdev.c
>>>> +++ b/net/switchdev/switchdev.c
>>>> @@ -31,3 +31,22 @@ int netdev_switch_parent_id_get(struct net_device
>>>> *dev,
>>>> return ops->ndo_switch_parent_id_get(dev, psid);
>>>> }
>>>> EXPORT_SYMBOL(netdev_switch_parent_id_get);
>>>> +
>>>> +/**
>>>> + * netdev_switch_port_stp_update - Notify switch device port of STP
>>>> + * state change
>>>> + * @dev: port device
>>>> + * @state: port STP state
>>>> + *
>>>> + * Notify switch device port of bridge port STP state change.
>>>> + */
>>>> +int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>>>> +{
>>>> + const struct net_device_ops *ops = dev->netdev_ops;
>>>> +
>>>> + if (!ops->ndo_switch_port_stp_update)
>>>> + return -EOPNOTSUPP;
>>>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>>>> + return ops->ndo_switch_port_stp_update(dev, state);
>>>> +}
>>>> +EXPORT_SYMBOL(netdev_switch_port_stp_update);
>>>
>>>
>>> This should also check if offload is enabled on the bridge/port ?
>>>
>
--
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
Powered by blists - more mailing lists