| Message ID | 1409736300-12303-8-git-send-email-jiri@resnulli.us |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 09/03/2014 02:24 AM, Jiri Pirko wrote: > Signed-off-by: Jiri Pirko <jiri@resnulli.us> > --- > include/linux/netdevice.h | 3 ++- > include/net/dsa.h | 1 + > net/dsa/Kconfig | 2 +- > net/dsa/dsa.c | 3 +++ > net/dsa/slave.c | 10 ++++++++++ > 5 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 6a009d1..7ee070f 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -41,7 +41,6 @@ > > #include <linux/ethtool.h> > #include <net/net_namespace.h> > -#include <net/dsa.h> > #ifdef CONFIG_DCB > #include <net/dcbnl.h> > #endif > @@ -1259,6 +1258,8 @@ enum netdev_priv_flags { > #define IFF_LIVE_ADDR_CHANGE IFF_LIVE_ADDR_CHANGE > #define IFF_MACVLAN IFF_MACVLAN > > +#include <net/dsa.h> > + > /** > * struct net_device - The DEVICE structure. > * Actually, this whole structure is a big mistake. It mixes I/O > diff --git a/include/net/dsa.h b/include/net/dsa.h > index 9771292..d60cd42 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -140,6 +140,7 @@ struct dsa_switch { > u32 phys_mii_mask; > struct mii_bus *slave_mii_bus; > struct net_device *ports[DSA_MAX_PORTS]; > + struct netdev_phys_item_id psid; > }; > > static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p) > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig > index a585fd6..4e144a2 100644 > --- a/net/dsa/Kconfig > +++ b/net/dsa/Kconfig > @@ -1,6 +1,6 @@ > config HAVE_NET_DSA > def_bool y > - depends on NETDEVICES && !S390 > + depends on NETDEVICES && NET_SWITCHDEV && !S390 It does not look like this is necessary, we are only using definitions from net/dsa.h and include/linux/netdevice.h, and if it was, a 'select' would be more appropriate here I think. TBH, I think we should rather drop this patch for now, I do not see any benefit in providing a random id over no-id at all. > > # Drivers must select NET_DSA and the appropriate tagging format > > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c > index 61f145c..374912d 100644 > --- a/net/dsa/dsa.c > +++ b/net/dsa/dsa.c > @@ -202,6 +202,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index, > ds->ports[i] = slave_dev; > } > > + ds->psid.id_len = MAX_PHYS_ITEM_ID_LEN; > + get_random_bytes(ds->psid.id, ds->psid.id_len); > + > return ds; > > out_free: > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index 7333a4a..d79a6c7 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -192,6 +192,15 @@ static netdev_tx_t dsa_slave_notag_xmit(struct sk_buff *skb, > return NETDEV_TX_OK; > } > > +static int dsa_slave_swdev_get_id(struct net_device *dev, > + struct netdev_phys_item_id *psid) > +{ > + struct dsa_slave_priv *p = netdev_priv(dev); > + struct dsa_switch *ds = p->parent; > + > + memcpy(psid, &ds->psid, sizeof(*psid)); > + return 0; > +} > > /* ethtool operations *******************************************************/ > static int > @@ -323,6 +332,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = { > .ndo_set_rx_mode = dsa_slave_set_rx_mode, > .ndo_set_mac_address = dsa_slave_set_mac_address, > .ndo_do_ioctl = dsa_slave_ioctl, > + .ndo_swdev_get_id = dsa_slave_swdev_get_id, > }; > > static const struct dsa_device_ops notag_netdev_ops = { > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thu, Sep 04, 2014 at 01:20:58AM CEST, f.fainelli@gmail.com wrote: >On 09/03/2014 02:24 AM, Jiri Pirko wrote: >> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >> --- >> include/linux/netdevice.h | 3 ++- >> include/net/dsa.h | 1 + >> net/dsa/Kconfig | 2 +- >> net/dsa/dsa.c | 3 +++ >> net/dsa/slave.c | 10 ++++++++++ >> 5 files changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index 6a009d1..7ee070f 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -41,7 +41,6 @@ >> >> #include <linux/ethtool.h> >> #include <net/net_namespace.h> >> -#include <net/dsa.h> >> #ifdef CONFIG_DCB >> #include <net/dcbnl.h> >> #endif >> @@ -1259,6 +1258,8 @@ enum netdev_priv_flags { >> #define IFF_LIVE_ADDR_CHANGE IFF_LIVE_ADDR_CHANGE >> #define IFF_MACVLAN IFF_MACVLAN >> >> +#include <net/dsa.h> >> + >> /** >> * struct net_device - The DEVICE structure. >> * Actually, this whole structure is a big mistake. It mixes I/O >> diff --git a/include/net/dsa.h b/include/net/dsa.h >> index 9771292..d60cd42 100644 >> --- a/include/net/dsa.h >> +++ b/include/net/dsa.h >> @@ -140,6 +140,7 @@ struct dsa_switch { >> u32 phys_mii_mask; >> struct mii_bus *slave_mii_bus; >> struct net_device *ports[DSA_MAX_PORTS]; >> + struct netdev_phys_item_id psid; >> }; >> >> static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p) >> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig >> index a585fd6..4e144a2 100644 >> --- a/net/dsa/Kconfig >> +++ b/net/dsa/Kconfig >> @@ -1,6 +1,6 @@ >> config HAVE_NET_DSA >> def_bool y >> - depends on NETDEVICES && !S390 >> + depends on NETDEVICES && NET_SWITCHDEV && !S390 > >It does not look like this is necessary, we are only using definitions >from net/dsa.h and include/linux/netdevice.h, and if it was, a 'select' >would be more appropriate here I think. > >TBH, I think we should rather drop this patch for now, I do not see any >benefit in providing a random id over no-id at all. Well, the benefit is that you are still able to see which ports belong to the same switch. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014-09-04 14:47, Jiri Pirko wrote: > Thu, Sep 04, 2014 at 01:20:58AM CEST, f.fainelli@gmail.com wrote: >>On 09/03/2014 02:24 AM, Jiri Pirko wrote: >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>> --- >>> include/linux/netdevice.h | 3 ++- >>> include/net/dsa.h | 1 + >>> net/dsa/Kconfig | 2 +- >>> net/dsa/dsa.c | 3 +++ >>> net/dsa/slave.c | 10 ++++++++++ >>> 5 files changed, 17 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>> index 6a009d1..7ee070f 100644 >>> --- a/include/linux/netdevice.h >>> +++ b/include/linux/netdevice.h >>> @@ -41,7 +41,6 @@ >>> >>> #include <linux/ethtool.h> >>> #include <net/net_namespace.h> >>> -#include <net/dsa.h> >>> #ifdef CONFIG_DCB >>> #include <net/dcbnl.h> >>> #endif >>> @@ -1259,6 +1258,8 @@ enum netdev_priv_flags { >>> #define IFF_LIVE_ADDR_CHANGE IFF_LIVE_ADDR_CHANGE >>> #define IFF_MACVLAN IFF_MACVLAN >>> >>> +#include <net/dsa.h> >>> + >>> /** >>> * struct net_device - The DEVICE structure. >>> * Actually, this whole structure is a big mistake. It mixes I/O >>> diff --git a/include/net/dsa.h b/include/net/dsa.h >>> index 9771292..d60cd42 100644 >>> --- a/include/net/dsa.h >>> +++ b/include/net/dsa.h >>> @@ -140,6 +140,7 @@ struct dsa_switch { >>> u32 phys_mii_mask; >>> struct mii_bus *slave_mii_bus; >>> struct net_device *ports[DSA_MAX_PORTS]; >>> + struct netdev_phys_item_id psid; >>> }; >>> >>> static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p) >>> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig >>> index a585fd6..4e144a2 100644 >>> --- a/net/dsa/Kconfig >>> +++ b/net/dsa/Kconfig >>> @@ -1,6 +1,6 @@ >>> config HAVE_NET_DSA >>> def_bool y >>> - depends on NETDEVICES && !S390 >>> + depends on NETDEVICES && NET_SWITCHDEV && !S390 >> >>It does not look like this is necessary, we are only using definitions >>from net/dsa.h and include/linux/netdevice.h, and if it was, a 'select' >>would be more appropriate here I think. >> >>TBH, I think we should rather drop this patch for now, I do not see any >>benefit in providing a random id over no-id at all. > > Well, the benefit is that you are still able to see which ports belong > to the same switch. I think it's a bad idea to force switchdev bloat onto DSA users just for that random id thing. - Felix -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fri, Sep 05, 2014 at 06:43:23AM CEST, nbd@openwrt.org wrote: >On 2014-09-04 14:47, Jiri Pirko wrote: >> Thu, Sep 04, 2014 at 01:20:58AM CEST, f.fainelli@gmail.com wrote: >>>On 09/03/2014 02:24 AM, Jiri Pirko wrote: >>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>>> --- >>>> include/linux/netdevice.h | 3 ++- >>>> include/net/dsa.h | 1 + >>>> net/dsa/Kconfig | 2 +- >>>> net/dsa/dsa.c | 3 +++ >>>> net/dsa/slave.c | 10 ++++++++++ >>>> 5 files changed, 17 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>>> index 6a009d1..7ee070f 100644 >>>> --- a/include/linux/netdevice.h >>>> +++ b/include/linux/netdevice.h >>>> @@ -41,7 +41,6 @@ >>>> >>>> #include <linux/ethtool.h> >>>> #include <net/net_namespace.h> >>>> -#include <net/dsa.h> >>>> #ifdef CONFIG_DCB >>>> #include <net/dcbnl.h> >>>> #endif >>>> @@ -1259,6 +1258,8 @@ enum netdev_priv_flags { >>>> #define IFF_LIVE_ADDR_CHANGE IFF_LIVE_ADDR_CHANGE >>>> #define IFF_MACVLAN IFF_MACVLAN >>>> >>>> +#include <net/dsa.h> >>>> + >>>> /** >>>> * struct net_device - The DEVICE structure. >>>> * Actually, this whole structure is a big mistake. It mixes I/O >>>> diff --git a/include/net/dsa.h b/include/net/dsa.h >>>> index 9771292..d60cd42 100644 >>>> --- a/include/net/dsa.h >>>> +++ b/include/net/dsa.h >>>> @@ -140,6 +140,7 @@ struct dsa_switch { >>>> u32 phys_mii_mask; >>>> struct mii_bus *slave_mii_bus; >>>> struct net_device *ports[DSA_MAX_PORTS]; >>>> + struct netdev_phys_item_id psid; >>>> }; >>>> >>>> static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p) >>>> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig >>>> index a585fd6..4e144a2 100644 >>>> --- a/net/dsa/Kconfig >>>> +++ b/net/dsa/Kconfig >>>> @@ -1,6 +1,6 @@ >>>> config HAVE_NET_DSA >>>> def_bool y >>>> - depends on NETDEVICES && !S390 >>>> + depends on NETDEVICES && NET_SWITCHDEV && !S390 >>> >>>It does not look like this is necessary, we are only using definitions >>>from net/dsa.h and include/linux/netdevice.h, and if it was, a 'select' >>>would be more appropriate here I think. >>> >>>TBH, I think we should rather drop this patch for now, I do not see any >>>benefit in providing a random id over no-id at all. >> >> Well, the benefit is that you are still able to see which ports belong >> to the same switch. >I think it's a bad idea to force switchdev bloat onto DSA users just for >that random id thing. Np. I will drop this. > >- Felix -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 6a009d1..7ee070f 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -41,7 +41,6 @@ #include <linux/ethtool.h> #include <net/net_namespace.h> -#include <net/dsa.h> #ifdef CONFIG_DCB #include <net/dcbnl.h> #endif @@ -1259,6 +1258,8 @@ enum netdev_priv_flags { #define IFF_LIVE_ADDR_CHANGE IFF_LIVE_ADDR_CHANGE #define IFF_MACVLAN IFF_MACVLAN +#include <net/dsa.h> + /** * struct net_device - The DEVICE structure. * Actually, this whole structure is a big mistake. It mixes I/O diff --git a/include/net/dsa.h b/include/net/dsa.h index 9771292..d60cd42 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -140,6 +140,7 @@ struct dsa_switch { u32 phys_mii_mask; struct mii_bus *slave_mii_bus; struct net_device *ports[DSA_MAX_PORTS]; + struct netdev_phys_item_id psid; }; static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p) diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index a585fd6..4e144a2 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -1,6 +1,6 @@ config HAVE_NET_DSA def_bool y - depends on NETDEVICES && !S390 + depends on NETDEVICES && NET_SWITCHDEV && !S390 # Drivers must select NET_DSA and the appropriate tagging format diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 61f145c..374912d 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -202,6 +202,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index, ds->ports[i] = slave_dev; } + ds->psid.id_len = MAX_PHYS_ITEM_ID_LEN; + get_random_bytes(ds->psid.id, ds->psid.id_len); + return ds; out_free: diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 7333a4a..d79a6c7 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -192,6 +192,15 @@ static netdev_tx_t dsa_slave_notag_xmit(struct sk_buff *skb, return NETDEV_TX_OK; } +static int dsa_slave_swdev_get_id(struct net_device *dev, + struct netdev_phys_item_id *psid) +{ + struct dsa_slave_priv *p = netdev_priv(dev); + struct dsa_switch *ds = p->parent; + + memcpy(psid, &ds->psid, sizeof(*psid)); + return 0; +} /* ethtool operations *******************************************************/ static int @@ -323,6 +332,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = { .ndo_set_rx_mode = dsa_slave_set_rx_mode, .ndo_set_mac_address = dsa_slave_set_mac_address, .ndo_do_ioctl = dsa_slave_ioctl, + .ndo_swdev_get_id = dsa_slave_swdev_get_id, }; static const struct dsa_device_ops notag_netdev_ops = {
Signed-off-by: Jiri Pirko <jiri@resnulli.us> --- include/linux/netdevice.h | 3 ++- include/net/dsa.h | 1 + net/dsa/Kconfig | 2 +- net/dsa/dsa.c | 3 +++ net/dsa/slave.c | 10 ++++++++++ 5 files changed, 17 insertions(+), 2 deletions(-)