| 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: <551953DE.90702@cumulusnetworks.com>
Date: Mon, 30 Mar 2015 06:47:10 -0700
From: roopa <roopa@...ulusnetworks.com>
To: Jiri Pirko <jiri@...nulli.us>
CC: sfeldma@...il.com, netdev@...r.kernel.org, linux@...ck-us.net,
f.fainelli@...il.com
Subject: Re: [PATCH net-next 04/18] switchdev: convert STP update to swdev
attr set
On 3/30/15, 4:54 AM, Jiri Pirko wrote:
> Mon, Mar 30, 2015 at 10:40:22AM CEST, sfeldma@...il.com wrote:
>> From: Scott Feldman <sfeldma@...il.com>
>>
>> STP update is just a writable port attribute, so convert swdev_port_stp_update
>> to an attr set.
>>
>> Signed-off-by: Scott Feldman <sfeldma@...il.com>
>> ---
>> drivers/net/ethernet/rocker/rocker.c | 16 +++++++++++++---
>> include/net/switchdev.h | 13 ++-----------
>> net/bridge/br_stp.c | 6 +++++-
>> net/dsa/slave.c | 19 ++++++++++++++++++-
>> net/switchdev/switchdev.c | 28 ----------------------------
>> 5 files changed, 38 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>> index 4bcc555..224f91d 100644
>> --- a/drivers/net/ethernet/rocker/rocker.c
>> +++ b/drivers/net/ethernet/rocker/rocker.c
>> @@ -4237,11 +4237,21 @@ static int rocker_port_attr_get(struct net_device *dev, struct swdev_attr *attr)
>> return 0;
>> }
>>
>> -static int rocker_port_swdev_port_stp_update(struct net_device *dev, u8 state)
>> +static int rocker_port_attr_set(struct net_device *dev, struct swdev_attr *attr)
>> {
>> struct rocker_port *rocker_port = netdev_priv(dev);
>> + int err = 0;
>> +
>> + switch (attr->attr) {
>> + case SWDEV_ATTR_PORT_STP_STATE:
>> + err = rocker_port_stp_update(rocker_port, attr->stp_state);
>> + break;
>> + default:
>> + err = -EOPNOTSUPP;
>> + break;
>> + }
>>
>> - return rocker_port_stp_update(rocker_port, state);
>> + return err;
>> }
>>
>> static int rocker_port_swdev_fib_ipv4_add(struct net_device *dev,
>> @@ -4271,7 +4281,7 @@ static int rocker_port_swdev_fib_ipv4_del(struct net_device *dev,
>>
>> static const struct swdev_ops rocker_port_swdev_ops = {
>> .swdev_port_attr_get = rocker_port_attr_get,
>> - .swdev_port_stp_update = rocker_port_swdev_port_stp_update,
>> + .swdev_port_attr_set = rocker_port_attr_set,
>> .swdev_fib_ipv4_add = rocker_port_swdev_fib_ipv4_add,
>> .swdev_fib_ipv4_del = rocker_port_swdev_fib_ipv4_del,
>> };
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index fd36b5c..99050b0 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -17,6 +17,7 @@
>> enum swdev_attr_id {
>> SWDEV_ATTR_UNDEFINED,
>> SWDEV_ATTR_PORT_PARENT_ID,
>> + SWDEV_ATTR_PORT_STP_STATE,
>> };
>>
>> #define SWDEV_ATTR_F_NO_RECURSE BIT(0)
>> @@ -27,6 +28,7 @@ struct swdev_attr {
>> u32 flags;
>> union {
>> struct netdev_phys_item_id ppid; /* PORT_PARENT_ID */
>> + u8 stp_state; /* PORT_STP_STATE */
>> };
>> };
>>
>> @@ -39,9 +41,6 @@ struct fib_info;
>> *
>> * @swdev_port_attr_set: Set a port attribute (see swdev_attr).
>> *
>> - * @swdev_port_stp_update: Called to notify switch device port of bridge
>> - * port STP state change.
>> - *
>> * @swdev_fib_ipv4_add: Called to add/modify IPv4 route to switch device.
>> *
>> * @swdev_fib_ipv4_del: Called to delete IPv4 route from switch device.
>> @@ -51,7 +50,6 @@ struct swdev_ops {
>> struct swdev_attr *attr);
>> int (*swdev_port_attr_set)(struct net_device *dev,
>> struct swdev_attr *attr);
>> - int (*swdev_port_stp_update)(struct net_device *dev, u8 state);
>> int (*swdev_fib_ipv4_add)(struct net_device *dev, __be32 dst,
>> int dst_len, struct fib_info *fi,
>> u8 tos, u8 type, u32 nlflags,
>> @@ -86,7 +84,6 @@ netdev_switch_notifier_info_to_dev(const struct netdev_switch_notifier_info *inf
>>
>> int swdev_port_attr_get(struct net_device *dev, struct swdev_attr *attr);
>> int swdev_port_attr_set(struct net_device *dev, struct swdev_attr *attr);
>> -int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>> int register_netdev_switch_notifier(struct notifier_block *nb);
>> int unregister_netdev_switch_notifier(struct notifier_block *nb);
>> int call_netdev_switch_notifiers(unsigned long val, struct net_device *dev,
>> @@ -119,12 +116,6 @@ static inline int swdev_port_attr_set(struct net_device *dev,
>> return -EOPNOTSUPP;
>> }
>>
>> -static inline int netdev_switch_port_stp_update(struct net_device *dev,
>> - u8 state)
>> -{
>> - return -EOPNOTSUPP;
>> -}
>> -
>> static inline int register_netdev_switch_notifier(struct notifier_block *nb)
>> {
>> return 0;
>> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
>> index fb3ebe6..8b2ef23 100644
>> --- a/net/bridge/br_stp.c
>> +++ b/net/bridge/br_stp.c
>> @@ -39,10 +39,14 @@ void br_log_state(const struct net_bridge_port *p)
>>
>> void br_set_state(struct net_bridge_port *p, unsigned int state)
>> {
>> + struct swdev_attr attr = {
>> + .attr = SWDEV_ATTR_PORT_STP_STATE,
>> + .stp_state = state,
>> + };
>> int err;
>>
>> p->state = state;
>> - err = netdev_switch_port_stp_update(p->dev, state);
>> + err = swdev_port_attr_set(p->dev, &attr);
> Seems nicer to me to provide a wrapper for callers so they don't call
> directly swdev_port_attr_set and have no knowledge of swdev_attr.
+1. I had a similar patch long back. swdev_port_attr_set can take a
attrname and void argument.
the caller only needs to know the attrname in that case.
I had used netlink attribute names at that time to not have another set
of duplicate attribute names in the swdev layer. But then the api has to
be called swdev_bridge_port_attr_set to match the bridge attribute
policy name.
but, i have no objections to having the new attribute names
SWDEV_ATTR_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