| Message ID | 1415530280-9190-4-git-send-email-jiri@resnulli.us |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 11/09/14 05:51, Jiri Pirko wrote: > The netdevice represents a port in a switch, it will expose > IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value > belong to one physical switch. > > Signed-off-by: Jiri Pirko <jiri@resnulli.us> > --- > include/uapi/linux/if_link.h | 1 + > net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index 7072d83..4163753 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -145,6 +145,7 @@ enum { > IFLA_CARRIER, > IFLA_PHYS_PORT_ID, > IFLA_CARRIER_CHANGES, > + IFLA_PHYS_SWITCH_ID, > __IFLA_MAX > }; > > @@ -1198,6 +1221,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { > [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, > [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, > [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */ > + [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, > }; > Ok, looking at this compared to #1 i can see you are introducing IFLA_PHYS_SWITCH_ID but then why did you need to change IFLA_PHYS_PORT_ID earlier? cheers, jamal > static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { > -- 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
Mon, Nov 10, 2014 at 04:43:58AM CET, jhs@mojatatu.com wrote: >On 11/09/14 05:51, Jiri Pirko wrote: >>The netdevice represents a port in a switch, it will expose >>IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value >>belong to one physical switch. >> >>Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>--- >> include/uapi/linux/if_link.h | 1 + >> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- >> 2 files changed, 26 insertions(+), 1 deletion(-) >> >>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >>index 7072d83..4163753 100644 >>--- a/include/uapi/linux/if_link.h >>+++ b/include/uapi/linux/if_link.h >>@@ -145,6 +145,7 @@ enum { >> IFLA_CARRIER, >> IFLA_PHYS_PORT_ID, >> IFLA_CARRIER_CHANGES, >>+ IFLA_PHYS_SWITCH_ID, >> __IFLA_MAX >> }; >> > > >>@@ -1198,6 +1221,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { >> [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, >> [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, >> [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */ >>+ [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, >> }; >> > >Ok, looking at this compared to #1 i can see you are introducing >IFLA_PHYS_SWITCH_ID but then why did you need to change IFLA_PHYS_PORT_ID >earlier? I did not change it at all. I only made the name more generic. Please look closer to the patch 1. > >cheers, >jamal > >> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { >> > -- 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 11/9/14, 2:51 AM, Jiri Pirko wrote: > The netdevice represents a port in a switch, it will expose > IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value > belong to one physical switch. > > Signed-off-by: Jiri Pirko <jiri@resnulli.us> > --- > include/uapi/linux/if_link.h | 1 + > net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index 7072d83..4163753 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -145,6 +145,7 @@ enum { > IFLA_CARRIER, > IFLA_PHYS_PORT_ID, > IFLA_CARRIER_CHANGES, > + IFLA_PHYS_SWITCH_ID, Jiri, since we have not really converged on the switchdev class or having a separate switchdev instance, am thinking it is better if we dont expose any such switch_id to userspace yet until absolutely needed. Do you need it today ? There is no real in kernel hw switch driver that will use it today. And quite likely this will need to change when we introduce real hw switch drivers. > __IFLA_MAX > }; > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 1087c6d..f839354 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -43,6 +43,7 @@ > > #include <linux/inet.h> > #include <linux/netdevice.h> > +#include <net/switchdev.h> > #include <net/ip.h> > #include <net/protocol.h> > #include <net/arp.h> > @@ -868,7 +869,8 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, > + rtnl_port_size(dev, ext_filter_mask) /* IFLA_VF_PORTS + IFLA_PORT_SELF */ > + rtnl_link_get_size(dev) /* IFLA_LINKINFO */ > + rtnl_link_get_af_size(dev) /* IFLA_AF_SPEC */ > - + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_PORT_ID */ > + + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */ > + + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_SWITCH_ID */ > } > > static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev) > @@ -967,6 +969,24 @@ static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev) > return 0; > } > > +static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev) > +{ > + int err; > + struct netdev_phys_item_id psid; > + > + err = netdev_sw_parent_id_get(dev, &psid); > + if (err) { > + if (err == -EOPNOTSUPP) > + return 0; > + return err; > + } > + > + if (nla_put(skb, IFLA_PHYS_SWITCH_ID, psid.id_len, psid.id)) > + return -EMSGSIZE; > + > + return 0; > +} > + > static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, > int type, u32 pid, u32 seq, u32 change, > unsigned int flags, u32 ext_filter_mask) > @@ -1039,6 +1059,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, > if (rtnl_phys_port_id_fill(skb, dev)) > goto nla_put_failure; > > + if (rtnl_phys_switch_id_fill(skb, dev)) > + goto nla_put_failure; > + > attr = nla_reserve(skb, IFLA_STATS, > sizeof(struct rtnl_link_stats)); > if (attr == NULL) > @@ -1198,6 +1221,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { > [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, > [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, > [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */ > + [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, > }; > > static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { -- 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 Mon, Nov 10, 2014 at 7:58 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On 11/9/14, 2:51 AM, Jiri Pirko wrote: >> >> The netdevice represents a port in a switch, it will expose >> IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value >> belong to one physical switch. >> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >> --- >> include/uapi/linux/if_link.h | 1 + >> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- >> 2 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >> index 7072d83..4163753 100644 >> --- a/include/uapi/linux/if_link.h >> +++ b/include/uapi/linux/if_link.h >> @@ -145,6 +145,7 @@ enum { >> IFLA_CARRIER, >> IFLA_PHYS_PORT_ID, >> IFLA_CARRIER_CHANGES, >> + IFLA_PHYS_SWITCH_ID, > > > Jiri, since we have not really converged on the switchdev class or having a > separate switchdev instance, > am thinking it is better if we dont expose any such switch_id to userspace > yet until absolutely needed. Do you need it today ? > There is no real in kernel hw switch driver that will use it today. And > quite likely this will need to change when we introduce real hw switch > drivers. How will it change when real hw switch drivers are introduced? Will the real sw driver not be able to give a up unique ID for the 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 11/09/2014 02:51 AM, Jiri Pirko wrote: > The netdevice represents a port in a switch, it will expose > IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value > belong to one physical switch. > > Signed-off-by: Jiri Pirko <jiri@resnulli.us> > --- Yep, I need something like this for my management app to learn how the switch is laid out. This becomes more relevant if I have switch silicon mixed with NICs that are not connected to the switch object in the same platform. Acked-by: John Fastabend <john.r.fastabend@intel.com>
Mon, Nov 10, 2014 at 06:58:08PM CET, roopa@cumulusnetworks.com wrote: >On 11/9/14, 2:51 AM, Jiri Pirko wrote: >>The netdevice represents a port in a switch, it will expose >>IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value >>belong to one physical switch. >> >>Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>--- >> include/uapi/linux/if_link.h | 1 + >> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- >> 2 files changed, 26 insertions(+), 1 deletion(-) >> >>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >>index 7072d83..4163753 100644 >>--- a/include/uapi/linux/if_link.h >>+++ b/include/uapi/linux/if_link.h >>@@ -145,6 +145,7 @@ enum { >> IFLA_CARRIER, >> IFLA_PHYS_PORT_ID, >> IFLA_CARRIER_CHANGES, >>+ IFLA_PHYS_SWITCH_ID, > >Jiri, since we have not really converged on the switchdev class or having a >separate switchdev instance, >am thinking it is better if we dont expose any such switch_id to userspace >yet until absolutely needed. Do you need it today ? >There is no real in kernel hw switch driver that will use it today. And quite >likely this will need to change when we introduce real hw switch drivers. When and if the switchdev class is introduced, switch id can happily live on. It is nothing against it. Userspace should use this id to group the ports of physical switch. > > >> __IFLA_MAX >> }; >>diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c >>index 1087c6d..f839354 100644 >>--- a/net/core/rtnetlink.c >>+++ b/net/core/rtnetlink.c >>@@ -43,6 +43,7 @@ >> #include <linux/inet.h> >> #include <linux/netdevice.h> >>+#include <net/switchdev.h> >> #include <net/ip.h> >> #include <net/protocol.h> >> #include <net/arp.h> >>@@ -868,7 +869,8 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, >> + rtnl_port_size(dev, ext_filter_mask) /* IFLA_VF_PORTS + IFLA_PORT_SELF */ >> + rtnl_link_get_size(dev) /* IFLA_LINKINFO */ >> + rtnl_link_get_af_size(dev) /* IFLA_AF_SPEC */ >>- + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_PORT_ID */ >>+ + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */ >>+ + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_SWITCH_ID */ >> } >> static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev) >>@@ -967,6 +969,24 @@ static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev) >> return 0; >> } >>+static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev) >>+{ >>+ int err; >>+ struct netdev_phys_item_id psid; >>+ >>+ err = netdev_sw_parent_id_get(dev, &psid); >>+ if (err) { >>+ if (err == -EOPNOTSUPP) >>+ return 0; >>+ return err; >>+ } >>+ >>+ if (nla_put(skb, IFLA_PHYS_SWITCH_ID, psid.id_len, psid.id)) >>+ return -EMSGSIZE; >>+ >>+ return 0; >>+} >>+ >> static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, >> int type, u32 pid, u32 seq, u32 change, >> unsigned int flags, u32 ext_filter_mask) >>@@ -1039,6 +1059,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, >> if (rtnl_phys_port_id_fill(skb, dev)) >> goto nla_put_failure; >>+ if (rtnl_phys_switch_id_fill(skb, dev)) >>+ goto nla_put_failure; >>+ >> attr = nla_reserve(skb, IFLA_STATS, >> sizeof(struct rtnl_link_stats)); >> if (attr == NULL) >>@@ -1198,6 +1221,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { >> [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, >> [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, >> [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */ >>+ [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, >> }; >> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { > -- 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 11/10/2014 02:14 PM, Jiri Pirko wrote: > Mon, Nov 10, 2014 at 06:58:08PM CET, roopa@cumulusnetworks.com wrote: >> On 11/9/14, 2:51 AM, Jiri Pirko wrote: >>> The netdevice represents a port in a switch, it will expose >>> IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value >>> belong to one physical switch. >>> >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>> --- >>> include/uapi/linux/if_link.h | 1 + >>> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- >>> 2 files changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >>> index 7072d83..4163753 100644 >>> --- a/include/uapi/linux/if_link.h >>> +++ b/include/uapi/linux/if_link.h >>> @@ -145,6 +145,7 @@ enum { >>> IFLA_CARRIER, >>> IFLA_PHYS_PORT_ID, >>> IFLA_CARRIER_CHANGES, >>> + IFLA_PHYS_SWITCH_ID, >> >> Jiri, since we have not really converged on the switchdev class or having a >> separate switchdev instance, >> am thinking it is better if we dont expose any such switch_id to userspace >> yet until absolutely needed. Do you need it today ? >> There is no real in kernel hw switch driver that will use it today. And quite >> likely this will need to change when we introduce real hw switch drivers. > > When and if the switchdev class is introduced, switch id can happily > live on. It is nothing against it. Userspace should use this id to group > the ports of physical switch. > > +1 I think we need this otherwise how will userspace "know" how the ports are related? If I have two switch silicon blocks in a single platform or perhaps have a switch silicon with traditional host nics on the same platform.
On 11/10/14, 12:02 PM, Scott Feldman wrote: > On Mon, Nov 10, 2014 at 7:58 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote: >> On 11/9/14, 2:51 AM, Jiri Pirko wrote: >>> The netdevice represents a port in a switch, it will expose >>> IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value >>> belong to one physical switch. >>> >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us> >>> --- >>> include/uapi/linux/if_link.h | 1 + >>> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- >>> 2 files changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >>> index 7072d83..4163753 100644 >>> --- a/include/uapi/linux/if_link.h >>> +++ b/include/uapi/linux/if_link.h >>> @@ -145,6 +145,7 @@ enum { >>> IFLA_CARRIER, >>> IFLA_PHYS_PORT_ID, >>> IFLA_CARRIER_CHANGES, >>> + IFLA_PHYS_SWITCH_ID, >> >> Jiri, since we have not really converged on the switchdev class or having a >> separate switchdev instance, >> am thinking it is better if we dont expose any such switch_id to userspace >> yet until absolutely needed. Do you need it today ? >> There is no real in kernel hw switch driver that will use it today. And >> quite likely this will need to change when we introduce real hw switch >> drivers. > How will it change when real hw switch drivers are introduced? Will > the real sw driver not be able to give a up unique ID for the switch? With my question i was trying to see if there are other ways to manage the relationship between the switch device and the ports, instead of an random id provided by each switch driver. Today the switch id namespace seems to be with each switch driver. On my systems on the first switch chip quite likely i will choose an id 0.,,and possibly some other driver will choose the same id. The switch id namespace handling was not clear to me. But, to your question, am sure we will have some id to go in there since the field is now available. Thanks, Roopa -- 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/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 7072d83..4163753 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -145,6 +145,7 @@ enum { IFLA_CARRIER, IFLA_PHYS_PORT_ID, IFLA_CARRIER_CHANGES, + IFLA_PHYS_SWITCH_ID, __IFLA_MAX }; diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 1087c6d..f839354 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -43,6 +43,7 @@ #include <linux/inet.h> #include <linux/netdevice.h> +#include <net/switchdev.h> #include <net/ip.h> #include <net/protocol.h> #include <net/arp.h> @@ -868,7 +869,8 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, + rtnl_port_size(dev, ext_filter_mask) /* IFLA_VF_PORTS + IFLA_PORT_SELF */ + rtnl_link_get_size(dev) /* IFLA_LINKINFO */ + rtnl_link_get_af_size(dev) /* IFLA_AF_SPEC */ - + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_PORT_ID */ + + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_PORT_ID */ + + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_SWITCH_ID */ } static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev) @@ -967,6 +969,24 @@ static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev) return 0; } +static int rtnl_phys_switch_id_fill(struct sk_buff *skb, struct net_device *dev) +{ + int err; + struct netdev_phys_item_id psid; + + err = netdev_sw_parent_id_get(dev, &psid); + if (err) { + if (err == -EOPNOTSUPP) + return 0; + return err; + } + + if (nla_put(skb, IFLA_PHYS_SWITCH_ID, psid.id_len, psid.id)) + return -EMSGSIZE; + + return 0; +} + static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, int type, u32 pid, u32 seq, u32 change, unsigned int flags, u32 ext_filter_mask) @@ -1039,6 +1059,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, if (rtnl_phys_port_id_fill(skb, dev)) goto nla_put_failure; + if (rtnl_phys_switch_id_fill(skb, dev)) + goto nla_put_failure; + attr = nla_reserve(skb, IFLA_STATS, sizeof(struct rtnl_link_stats)); if (attr == NULL) @@ -1198,6 +1221,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, [IFLA_CARRIER_CHANGES] = { .type = NLA_U32 }, /* ignored */ + [IFLA_PHYS_SWITCH_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_ITEM_ID_LEN }, }; static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
The netdevice represents a port in a switch, it will expose IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value belong to one physical switch. Signed-off-by: Jiri Pirko <jiri@resnulli.us> --- include/uapi/linux/if_link.h | 1 + net/core/rtnetlink.c | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)