Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski : On Tue, 7 Jun 2022 21:39:55 -0700 you wrote: > Netdev reference helpers have a dev_ prefix for historic > reasons. Renaming the old helpers would be too much churn > but we can rename the tracking ones which are relatively > recent and should be the default for new code. > > Rename: > dev_hold_track()-> netdev_hold() > dev_put_track() -> netdev_put() > dev_replace_track() -> netdev_ref_replace() > > [...] Here is the summary with links: - [net-next] net: rename reference+tracking helpers https://git.kernel.org/netdev/net-next/c/d62607c3fe45 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On Thu, Jun 9, 2022 at 4:50 AM Paolo Abeni wrote: > > On Wed, 2022-06-08 at 16:00 -0700, Eric Dumazet wrote: > > On Wed, Jun 8, 2022 at 3:58 PM David Ahern wrote: > > > > > > On 6/8/22 8:58 AM, Jakub Kicinski wrote: > > > > IMO to encourage use of the track-capable API we could keep their names > > > > short and call the legacy functions __netdev_hold() as I mentioned or > > > > maybe netdev_hold_notrack(). > > > > > > I like that option. Similar to the old nla_parse functions that were > > > renamed with _deprecated - makes it easier to catch new uses. > > > > I think we need to clearly document the needed conversions for future > > bugfix backports. > > > > To be on the same page: do you think we need something under > Documentation with this patch? or with the later dev_hold rename? or > did I misunderstood completely? Adding instructions in the comments describing the functions would probably help stable teams (or ourselves because they will ask us to take care of conflicts) And backport the dev_put()/dev_hold() rename to kernels without CONFIG_NET_DEV_REFCNT_TRACKER infra. s/dev_put()/netdev_put_notrack()/ s/dev_hold()/netdev_hold_notrack()/ ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On 6/8/22 8:58 AM, Jakub Kicinski wrote: > IMO to encourage use of the track-capable API we could keep their names > short and call the legacy functions __netdev_hold() as I mentioned or > maybe netdev_hold_notrack(). I like that option. Similar to the old nla_parse functions that were renamed with _deprecated - makes it easier to catch new uses. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On Wed, Jun 8, 2022 at 3:58 PM David Ahern wrote: > > On 6/8/22 8:58 AM, Jakub Kicinski wrote: > > IMO to encourage use of the track-capable API we could keep their names > > short and call the legacy functions __netdev_hold() as I mentioned or > > maybe netdev_hold_notrack(). > > I like that option. Similar to the old nla_parse functions that were > renamed with _deprecated - makes it easier to catch new uses. I think we need to clearly document the needed conversions for future bugfix backports. Alternative would be to _backport_ the renaming for all stable versions ;) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On Wed, 2022-06-08 at 16:00 -0700, Eric Dumazet wrote: > On Wed, Jun 8, 2022 at 3:58 PM David Ahern wrote: > > > > On 6/8/22 8:58 AM, Jakub Kicinski wrote: > > > IMO to encourage use of the track-capable API we could keep their names > > > short and call the legacy functions __netdev_hold() as I mentioned or > > > maybe netdev_hold_notrack(). > > > > I like that option. Similar to the old nla_parse functions that were > > renamed with _deprecated - makes it easier to catch new uses. > > I think we need to clearly document the needed conversions for future > bugfix backports. > To be on the same page: do you think we need something under Documentation with this patch? or with the later dev_hold rename? or did I misunderstood completely? Thanks! Paolo ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
Wed, Jun 08, 2022 at 04:58:27PM CEST, k...@kernel.org wrote: >On Wed, 8 Jun 2022 10:27:15 +0200 Jiri Pirko wrote: >> Wed, Jun 08, 2022 at 06:39:55AM CEST, k...@kernel.org wrote: >> >Netdev reference helpers have a dev_ prefix for historic >> >reasons. Renaming the old helpers would be too much churn >> >> Hmm, I think it would be great to eventually rename the rest too in >> order to maintain unique prefix for netdev things. Why do you think the >> "churn" would be an issue? > >Felt like we're better of moving everyone to the new tracking helpers >than doing just a pure rename. But I'm not opposed to a pure rename. > >> >diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c >> >index 817577e713d7..815738c0e067 100644 >> >--- a/drivers/net/macsec.c >> >+++ b/drivers/net/macsec.c >> >@@ -3462,7 +3462,7 @@ static int macsec_dev_init(struct net_device *dev) >> >memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len); >> > >> >/* Get macsec's reference to real_dev */ >> >- dev_hold_track(real_dev, >dev_tracker, GFP_KERNEL); >> >+ netdev_hold(real_dev, >dev_tracker, GFP_KERNEL); >> >> So we later decide to rename dev_hold() to obey the netdev_*() naming >> scheme, we would have collision. > >dev_hold() should not be used in new code, we should use tracking >everywhere. Given that we can name the old helpers __netdev_hold(). > >> Also, seems to me odd to have: >> OLDPREFIX_x() >> and >> NEWPREFIX_x() >> to be different functions. >> >> For the sake of not making naming mess, could we rather have: >> netdev_hold_track() >> or >> netdev_hold_tr() if the prior is too long >> ? > >See above, one day non-track version should be removed. >IMO to encourage use of the track-capable API we could keep their names >short and call the legacy functions __netdev_hold() as I mentioned or >maybe netdev_hold_notrack(). Okay, that makes sense. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On Wed, 8 Jun 2022 16:58:08 -0600 David Ahern wrote: > On 6/8/22 8:58 AM, Jakub Kicinski wrote: > > IMO to encourage use of the track-capable API we could keep their names > > short and call the legacy functions __netdev_hold() as I mentioned or > > maybe netdev_hold_notrack(). > > I like that option. Similar to the old nla_parse functions that were > renamed with _deprecated - makes it easier to catch new uses. Well, not really a perfect parallel because _deprecated nla has to stay forever, given it behaves differently, while _notrack would hopefully die either thru conversion or someone rightly taking an axe to the cobwebbed code. Either way, I hope nobody is against merging the current patch. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
Wed, Jun 08, 2022 at 06:39:55AM CEST, k...@kernel.org wrote: >Netdev reference helpers have a dev_ prefix for historic >reasons. Renaming the old helpers would be too much churn Hmm, I think it would be great to eventually rename the rest too in order to maintain unique prefix for netdev things. Why do you think the "churn" would be an issue? >but we can rename the tracking ones which are relatively >recent and should be the default for new code. > >Rename: > dev_hold_track()-> netdev_hold() > dev_put_track() -> netdev_put() > dev_replace_track() -> netdev_ref_replace() [...] >diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c >index 817577e713d7..815738c0e067 100644 >--- a/drivers/net/macsec.c >+++ b/drivers/net/macsec.c >@@ -3462,7 +3462,7 @@ static int macsec_dev_init(struct net_device *dev) > memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len); > > /* Get macsec's reference to real_dev */ >- dev_hold_track(real_dev, >dev_tracker, GFP_KERNEL); >+ netdev_hold(real_dev, >dev_tracker, GFP_KERNEL); So we later decide to rename dev_hold() to obey the netdev_*() naming scheme, we would have collision. Also, seems to me odd to have: OLDPREFIX_x() and NEWPREFIX_x() to be different functions. For the sake of not making naming mess, could we rather have: netdev_hold_track() or netdev_hold_tr() if the prior is too long ? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
On Wed, 8 Jun 2022 10:27:15 +0200 Jiri Pirko wrote: > Wed, Jun 08, 2022 at 06:39:55AM CEST, k...@kernel.org wrote: > >Netdev reference helpers have a dev_ prefix for historic > >reasons. Renaming the old helpers would be too much churn > > Hmm, I think it would be great to eventually rename the rest too in > order to maintain unique prefix for netdev things. Why do you think the > "churn" would be an issue? Felt like we're better of moving everyone to the new tracking helpers than doing just a pure rename. But I'm not opposed to a pure rename. > >diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c > >index 817577e713d7..815738c0e067 100644 > >--- a/drivers/net/macsec.c > >+++ b/drivers/net/macsec.c > >@@ -3462,7 +3462,7 @@ static int macsec_dev_init(struct net_device *dev) > > memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len); > > > > /* Get macsec's reference to real_dev */ > >-dev_hold_track(real_dev, >dev_tracker, GFP_KERNEL); > >+netdev_hold(real_dev, >dev_tracker, GFP_KERNEL); > > So we later decide to rename dev_hold() to obey the netdev_*() naming > scheme, we would have collision. dev_hold() should not be used in new code, we should use tracking everywhere. Given that we can name the old helpers __netdev_hold(). > Also, seems to me odd to have: > OLDPREFIX_x() > and > NEWPREFIX_x() > to be different functions. > > For the sake of not making naming mess, could we rather have: > netdev_hold_track() > or > netdev_hold_tr() if the prior is too long > ? See above, one day non-track version should be removed. IMO to encourage use of the track-capable API we could keep their names short and call the legacy functions __netdev_hold() as I mentioned or maybe netdev_hold_notrack(). ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net-next] net: rename reference+tracking helpers
Netdev reference helpers have a dev_ prefix for historic reasons. Renaming the old helpers would be too much churn but we can rename the tracking ones which are relatively recent and should be the default for new code. Rename: dev_hold_track()-> netdev_hold() dev_put_track() -> netdev_put() dev_replace_track() -> netdev_ref_replace() Signed-off-by: Jakub Kicinski --- CC: dsah...@kernel.org CC: steffen.klass...@secunet.com CC: jreu...@yaina.de CC: ra...@blackwall.org CC: j...@resnulli.us CC: kgr...@linux.ibm.com CC: ivec...@redhat.com CC: jma...@redhat.com CC: ying@windriver.com CC: lucien@gmail.com CC: a...@arndb.de CC: yajun.d...@linux.dev CC: aten...@kernel.org CC: richardsonn...@google.com CC: hkallwe...@gmail.com CC: linux-h...@vger.kernel.org CC: d...@openvswitch.org CC: linux-s...@vger.kernel.org CC: tipc-discuss...@lists.sourceforge.net --- drivers/net/eql.c | 4 ++-- drivers/net/macsec.c | 4 ++-- drivers/net/macvlan.c | 4 ++-- drivers/net/netconsole.c | 2 +- drivers/net/vrf.c | 8 include/linux/netdevice.h | 24 include/net/xfrm.h | 2 +- net/8021q/vlan_dev.c | 4 ++-- net/ax25/af_ax25.c | 7 --- net/ax25/ax25_dev.c| 6 +++--- net/bridge/br_if.c | 10 +- net/core/dev.c | 10 +- net/core/dev_ioctl.c | 4 ++-- net/core/drop_monitor.c| 5 +++-- net/core/dst.c | 8 net/core/failover.c| 4 ++-- net/core/link_watch.c | 2 +- net/core/neighbour.c | 18 +- net/core/net-sysfs.c | 8 net/core/netpoll.c | 2 +- net/core/pktgen.c | 6 +++--- net/ethtool/ioctl.c| 4 ++-- net/ethtool/netlink.c | 6 +++--- net/ethtool/netlink.h | 2 +- net/ipv4/devinet.c | 4 ++-- net/ipv4/fib_semantics.c | 11 ++- net/ipv4/ipmr.c| 2 +- net/ipv4/route.c | 7 +++ net/ipv4/xfrm4_policy.c| 2 +- net/ipv6/addrconf.c| 4 ++-- net/ipv6/addrconf_core.c | 2 +- net/ipv6/ip6_gre.c | 8 net/ipv6/ip6_tunnel.c | 4 ++-- net/ipv6/ip6_vti.c | 4 ++-- net/ipv6/ip6mr.c | 2 +- net/ipv6/route.c | 10 +- net/ipv6/sit.c | 4 ++-- net/ipv6/xfrm6_policy.c| 4 ++-- net/llc/af_llc.c | 2 +- net/openvswitch/vport-netdev.c | 6 +++--- net/packet/af_packet.c | 12 ++-- net/sched/act_mirred.c | 6 +++--- net/sched/sch_api.c| 2 +- net/sched/sch_generic.c| 11 ++- net/smc/smc_pnet.c | 7 --- net/switchdev/switchdev.c | 4 ++-- net/tipc/bearer.c | 4 ++-- net/xfrm/xfrm_device.c | 2 +- 48 files changed, 141 insertions(+), 137 deletions(-) diff --git a/drivers/net/eql.c b/drivers/net/eql.c index 557ca8ff9dec..ca3e4700a813 100644 --- a/drivers/net/eql.c +++ b/drivers/net/eql.c @@ -225,7 +225,7 @@ static void eql_kill_one_slave(slave_queue_t *queue, slave_t *slave) list_del(>list); queue->num_slaves--; slave->dev->flags &= ~IFF_SLAVE; - dev_put_track(slave->dev, >dev_tracker); + netdev_put(slave->dev, >dev_tracker); kfree(slave); } @@ -399,7 +399,7 @@ static int __eql_insert_slave(slave_queue_t *queue, slave_t *slave) if (duplicate_slave) eql_kill_one_slave(queue, duplicate_slave); - dev_hold_track(slave->dev, >dev_tracker, GFP_ATOMIC); + netdev_hold(slave->dev, >dev_tracker, GFP_ATOMIC); list_add(>list, >all_slaves); queue->num_slaves++; slave->dev->flags |= IFF_SLAVE; diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 817577e713d7..815738c0e067 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -3462,7 +3462,7 @@ static int macsec_dev_init(struct net_device *dev) memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len); /* Get macsec's reference to real_dev */ - dev_hold_track(real_dev, >dev_tracker, GFP_KERNEL); + netdev_hold(real_dev, >dev_tracker, GFP_KERNEL); return 0; } @@ -3710,7 +3710,7 @@ static void macsec_free_netdev(struct net_device *dev) free_percpu(macsec->secy.tx_sc.stats); /* Get rid of the macsec's reference to real_dev */ - dev_put_track(macsec->real_dev, >dev_tracker); + netdev_put(macsec->real_dev, >dev_tracker); } static void macsec_setup(struct net_device *dev) diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index eff75beb1395..5b46a6526fc6 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -915,7 +915,7 @@ static int macvlan_init(struct
