From: roopa <roopa@cumulusnetworks•com>
To: Jiri Pirko <jiri@resnulli•us>
Cc: sfeldma@gmail•com, netdev@vger•kernel.org, linux@roeck-us•net,
f.fainelli@gmail•com
Subject: Re: [PATCH net-next 04/18] switchdev: convert STP update to swdev attr set
Date: Mon, 30 Mar 2015 06:47:10 -0700 [thread overview]
Message-ID: <551953DE.90702@cumulusnetworks.com> (raw)
In-Reply-To: <20150330115417.GC2045@nanopsycho.orion>
On 3/30/15, 4:54 AM, Jiri Pirko wrote:
> Mon, Mar 30, 2015 at 10:40:22AM CEST, sfeldma@gmail•com wrote:
>> From: Scott Feldman <sfeldma@gmail•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@gmail•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_*
next prev parent reply other threads:[~2015-03-30 13:47 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-30 8:40 [PATCH net-next 00/18] switchdev: spring cleanup sfeldma
2015-03-30 8:40 ` [PATCH net-next 01/18] switchdev: introduce get/set attrs ops sfeldma
2015-03-30 8:40 ` [PATCH net-next 02/18] switchdev: flesh out get/set attr ops sfeldma
2015-03-30 11:55 ` Jiri Pirko
2015-03-30 18:32 ` Arad, Ronen
2015-03-30 20:46 ` Jiri Pirko
2015-03-30 21:00 ` Scott Feldman
2015-03-31 0:22 ` Arad, Ronen
2015-03-31 0:38 ` Scott Feldman
2015-03-31 15:37 ` roopa
[not found] ` <CAJieiUh0Svt3LZsgoi7RaV8Be0eFyRvoJU3BmW7v3fEvwfXiHg@mail.gmail.com>
2015-03-31 16:05 ` Scott Feldman
2015-03-30 8:40 ` [PATCH net-next 03/18] switchdev: convert parent_id_get to swdev attr get sfeldma
2015-03-30 8:40 ` [PATCH net-next 04/18] switchdev: convert STP update to swdev attr set sfeldma
2015-03-30 11:54 ` Jiri Pirko
2015-03-30 13:47 ` roopa [this message]
2015-03-30 8:40 ` [PATCH net-next 05/18] switchdev: add bridge port flags attr sfeldma
2015-03-30 8:40 ` [PATCH net-next 06/18] rocker: use swdev get/set attr for bridge port flags sfeldma
2015-03-30 12:01 ` Jiri Pirko
2015-03-30 8:40 ` [PATCH net-next 07/18] switchdev: add new swdev bridge setlink sfeldma
2015-03-30 12:31 ` Jiri Pirko
2015-03-30 8:40 ` [PATCH net-next 08/18] rocker: cut over to new swdev_port_bridge_setlink sfeldma
2015-03-30 8:40 ` [PATCH net-next 09/18] bonding: " sfeldma
2015-03-30 8:40 ` [PATCH net-next 10/18] team: " sfeldma
2015-03-30 8:40 ` [PATCH net-next 11/18] switchdev: remove old netdev_switch_port_bridge_setlink sfeldma
2015-03-30 13:23 ` roopa
2015-03-30 20:20 ` Scott Feldman
2015-03-30 20:46 ` Arad, Ronen
2015-03-30 21:27 ` Scott Feldman
2015-03-31 0:08 ` Arad, Ronen
2015-03-31 0:44 ` Scott Feldman
2015-03-31 5:52 ` Jiri Pirko
2015-03-31 19:15 ` Arad, Ronen
2015-03-31 21:52 ` Jiri Pirko
2015-03-31 23:32 ` Arad, Ronen
2015-04-01 2:38 ` Scott Feldman
2015-04-01 12:03 ` Jamal Hadi Salim
2015-04-01 17:56 ` Scott Feldman
2015-04-02 1:01 ` Florian Fainelli
2015-03-30 8:40 ` [PATCH net-next 12/18] switchdev: remove unused netdev_switch_port_bridge_dellink sfeldma
2015-03-30 13:23 ` roopa
2015-03-30 8:40 ` [PATCH net-next 13/18] switchdev: remove unused NETIF_F_HW_SWITCH_OFFLOAD sfeldma
2015-03-30 13:38 ` roopa
2015-03-30 20:48 ` Samudrala, Sridhar
2015-03-30 21:20 ` Scott Feldman
2015-03-31 15:34 ` roopa
[not found] ` <CAJieiUiL4QRQAC30=bkYadYD2L2cOcn7mNLV98uH3Go0exMO+A@mail.gmail.com>
2015-03-31 16:03 ` Scott Feldman
2015-03-30 8:40 ` [PATCH net-next 14/18] switchdev: add new swdev_port_bridge_getlink sfeldma
2015-03-30 8:40 ` [PATCH net-next 15/18] rocker: cut over to " sfeldma
2015-03-30 8:40 ` [PATCH net-next 16/18] switchdev: rename netdev_switch_fib_* to swdev_fib_* sfeldma
2015-03-30 8:40 ` [PATCH net-next 17/18] switchdev: rename netdev_switch_notifier_* to swdev_notifier_* sfeldma
2015-03-30 8:40 ` [PATCH net-next 18/18] switchdev: bring documentation up-to-date sfeldma
2015-03-30 12:00 ` [PATCH net-next 00/18] switchdev: spring cleanup Jiri Pirko
2015-03-30 13:11 ` Andy Gospodarek
2015-03-30 15:00 ` roopa
2015-03-30 16:11 ` Or Gerlitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=551953DE.90702@cumulusnetworks.com \
--to=roopa@cumulusnetworks$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=linux@roeck-us$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sfeldma@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox