Message ID | 20190301081402.6074-6-jiri@resnulli.us |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | net: call for phys_port_name into devlink directly is possible | expand |
On Fri, 1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote: > From: Jiri Pirko <jiri@mellanox.com> > > Now it is unused, remove it. > > Signed-off-by: Jiri Pirko <jiri@mellanox.com> Oh, you nuke this completely, that's going to conflict hard with my series :) (Provided the locking is okay) I think it'd be good if we flipped the logic in dev_get_phys_port_name(). Always try devlink: @@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev, const struct net_device_ops *ops = dev->netdev_ops; + if (!devlink_compat_phys_port_name_get(dev, name, len)) + return 0; if (!ops->ndo_get_phys_port_name) return -EOPNOTSUPP; return ops->ndo_get_phys_port_name(dev, name, len); } NFP is using a single set of NDOs for PFs and VFs (which don't have devlink ports), so either it'd be good if I could call the devlink helper for real ports, or we need the above. > diff --git a/include/net/devlink.h b/include/net/devlink.h > index ae2cd34da99e..227c453cc20e 100644 > --- a/include/net/devlink.h > +++ b/include/net/devlink.h > @@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port, > enum devlink_port_flavour flavour, > u32 port_number, bool split, > u32 split_subport_number); > -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, > - char *name, size_t len); > int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, > u32 size, u16 ingress_pools_count, > u16 egress_pools_count, u16 ingress_tc_count, > @@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port, > { > } > > -static inline int > -devlink_port_get_phys_port_name(struct devlink_port *devlink_port, > - char *name, size_t len) > -{ > - return -EOPNOTSUPP; > -} > - > static inline int devlink_sb_register(struct devlink *devlink, > unsigned int sb_index, u32 size, > u16 ingress_pools_count, > diff --git a/net/core/devlink.c b/net/core/devlink.c > index 4fd45d4a4818..d90a745d8258 100644 > --- a/net/core/devlink.c > +++ b/net/core/devlink.c > @@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port, > return 0; > } > > -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, > - char *name, size_t len) > -{ > - return __devlink_port_phys_port_name_get(devlink_port, name, len); > -} > -EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name); > - > int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, > u32 size, u16 ingress_pools_count, > u16 egress_pools_count, u16 ingress_tc_count,
Fri, Mar 01, 2019 at 05:25:29PM CET, jakub.kicinski@netronome.com wrote: >On Fri, 1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@mellanox.com> >> >> Now it is unused, remove it. >> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > >Oh, you nuke this completely, that's going to conflict hard with my >series :) > >(Provided the locking is okay) I think it'd be good if we flipped the >logic in dev_get_phys_port_name(). Always try devlink: > >@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev, > const struct net_device_ops *ops = dev->netdev_ops; > >+ if (!devlink_compat_phys_port_name_get(dev, name, len)) >+ return 0; > if (!ops->ndo_get_phys_port_name) > return -EOPNOTSUPP; > return ops->ndo_get_phys_port_name(dev, name, len); > } > >NFP is using a single set of NDOs for PFs and VFs (which don't have >devlink ports), so either it'd be good if I could call the devlink >helper for real ports, or we need the above. That wouldn't give correct results now. Until you introduce PF/VF port flavours, devlink_compat_phys_port_name_get() does not assemble correct names for them. So we have to call ndo_get_phys_port_name until it knows how to do that. > >> diff --git a/include/net/devlink.h b/include/net/devlink.h >> index ae2cd34da99e..227c453cc20e 100644 >> --- a/include/net/devlink.h >> +++ b/include/net/devlink.h >> @@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port, >> enum devlink_port_flavour flavour, >> u32 port_number, bool split, >> u32 split_subport_number); >> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, >> - char *name, size_t len); >> int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, >> u32 size, u16 ingress_pools_count, >> u16 egress_pools_count, u16 ingress_tc_count, >> @@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port, >> { >> } >> >> -static inline int >> -devlink_port_get_phys_port_name(struct devlink_port *devlink_port, >> - char *name, size_t len) >> -{ >> - return -EOPNOTSUPP; >> -} >> - >> static inline int devlink_sb_register(struct devlink *devlink, >> unsigned int sb_index, u32 size, >> u16 ingress_pools_count, >> diff --git a/net/core/devlink.c b/net/core/devlink.c >> index 4fd45d4a4818..d90a745d8258 100644 >> --- a/net/core/devlink.c >> +++ b/net/core/devlink.c >> @@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port, >> return 0; >> } >> >> -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, >> - char *name, size_t len) >> -{ >> - return __devlink_port_phys_port_name_get(devlink_port, name, len); >> -} >> -EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name); >> - >> int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, >> u32 size, u16 ingress_pools_count, >> u16 egress_pools_count, u16 ingress_tc_count, >
On Fri, 1 Mar 2019 17:27:20 +0100, Jiri Pirko wrote: > Fri, Mar 01, 2019 at 05:25:29PM CET, jakub.kicinski@netronome.com wrote: > >On Fri, 1 Mar 2019 09:14:02 +0100, Jiri Pirko wrote: > >> From: Jiri Pirko <jiri@mellanox.com> > >> > >> Now it is unused, remove it. > >> > >> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > > > >Oh, you nuke this completely, that's going to conflict hard with my > >series :) > > > >(Provided the locking is okay) I think it'd be good if we flipped the > >logic in dev_get_phys_port_name(). Always try devlink: > > > >@@ -7872,7 +7873,7 @@ int dev_get_phys_port_name(struct net_device *dev, > > const struct net_device_ops *ops = dev->netdev_ops; > > > >+ if (!devlink_compat_phys_port_name_get(dev, name, len)) > >+ return 0; > > if (!ops->ndo_get_phys_port_name) > > return -EOPNOTSUPP; > > return ops->ndo_get_phys_port_name(dev, name, len); > > } > > > >NFP is using a single set of NDOs for PFs and VFs (which don't have > >devlink ports), so either it'd be good if I could call the devlink > >helper for real ports, or we need the above. > > That wouldn't give correct results now. Until you introduce PF/VF port > flavours, devlink_compat_phys_port_name_get() does not assemble correct > names for them. So we have to call ndo_get_phys_port_name until it knows > how to do that. Okay, yeah, this won't give correct results after my series though, so please advise on which order you want these things merged? I don't think there were any changes or concerns with my series - one commit message fix, split one helper into two, and wrap the peer into another attr in the dump? Can it go in first?
diff --git a/include/net/devlink.h b/include/net/devlink.h index ae2cd34da99e..227c453cc20e 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -578,8 +578,6 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port, enum devlink_port_flavour flavour, u32 port_number, bool split, u32 split_subport_number); -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, - char *name, size_t len); int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, u32 size, u16 ingress_pools_count, u16 egress_pools_count, u16 ingress_tc_count, @@ -794,13 +792,6 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port, { } -static inline int -devlink_port_get_phys_port_name(struct devlink_port *devlink_port, - char *name, size_t len) -{ - return -EOPNOTSUPP; -} - static inline int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, u32 size, u16 ingress_pools_count, diff --git a/net/core/devlink.c b/net/core/devlink.c index 4fd45d4a4818..d90a745d8258 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -5451,13 +5451,6 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port, return 0; } -int devlink_port_get_phys_port_name(struct devlink_port *devlink_port, - char *name, size_t len) -{ - return __devlink_port_phys_port_name_get(devlink_port, name, len); -} -EXPORT_SYMBOL_GPL(devlink_port_get_phys_port_name); - int devlink_sb_register(struct devlink *devlink, unsigned int sb_index, u32 size, u16 ingress_pools_count, u16 egress_pools_count, u16 ingress_tc_count,