| Message ID | 20240514091306.229444-1-lukma@denx.de (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: hsr: Setup and delete proxy prune timer only when RedBox is enabled | expand |
On 2024-05-14 11:13:06 [+0200], Lukasz Majewski wrote: > The timer for removing entries in the ProxyNodeTable shall be only active > when the HSR driver works as RedBox (HSR-SAN). > > Moreover, the obsolete del_timer_sync() is replaced with > timer_delete_sync(). > > This patch improves fix from commit 3c668cef61ad > ("net: hsr: init prune_proxy_timer sooner") as the prune node > timer shall be setup only when HSR RedBox is supported in the node. Is it problematic to init/ delete the timer in non-redbox mode? It looks easier and it is not a hotpath. > Signed-off-by: Lukasz Majewski <lukma@denx.de> Sebastian
Hi Sebastian, > On 2024-05-14 11:13:06 [+0200], Lukasz Majewski wrote: > > The timer for removing entries in the ProxyNodeTable shall be only > > active when the HSR driver works as RedBox (HSR-SAN). > > > > Moreover, the obsolete del_timer_sync() is replaced with > > timer_delete_sync(). > > > > This patch improves fix from commit 3c668cef61ad > > ("net: hsr: init prune_proxy_timer sooner") as the prune node > > timer shall be setup only when HSR RedBox is supported in the node. > > > > Is it problematic to init/ delete the timer in non-redbox mode? It > looks easier and it is not a hotpath. My concern is only with resource allocation - when RedBox is not enabled the resources for this particular, not used timer are allocated anyway. If this can be omitted - then we can drop the patch. > > > Signed-off-by: Lukasz Majewski <lukma@denx.de> > > Sebastian Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On 2024-05-15 09:09:04 [+0200], Lukasz Majewski wrote: > Hi Sebastian, Hi Lukasz, > My concern is only with resource allocation - when RedBox is not > enabled the resources for this particular, not used timer are allocated > anyway. timer_setup() does not allocate any resources. The initialisation is pure static assignment. The timer subsystem does not look at this timer until mod_timer() is invoked (or something similar). > If this can be omitted - then we can drop the patch. > > Best regards, > > Lukasz Majewski Sebastian
Hi Sebastian, > On 2024-05-15 09:09:04 [+0200], Lukasz Majewski wrote: > > Hi Sebastian, > Hi Lukasz, > > > My concern is only with resource allocation - when RedBox is not > > enabled the resources for this particular, not used timer are > > allocated anyway. > > timer_setup() does not allocate any resources. The initialisation is > pure static assignment. The timer subsystem does not look at this > timer until mod_timer() is invoked (or something similar). Thanks for the clarification. Considering the above - please just drop this patch. > > > If this can be omitted - then we can drop the patch. > > > > Best regards, > > > > Lukasz Majewski > > Sebastian Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c index e6904288d40d..d234e4134a9d 100644 --- a/net/hsr/hsr_device.c +++ b/net/hsr/hsr_device.c @@ -589,7 +589,6 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], timer_setup(&hsr->announce_timer, hsr_announce, 0); timer_setup(&hsr->prune_timer, hsr_prune_nodes, 0); - timer_setup(&hsr->prune_proxy_timer, hsr_prune_proxy_nodes, 0); ether_addr_copy(hsr->sup_multicast_addr, def_multicast_addr); hsr->sup_multicast_addr[ETH_ALEN - 1] = multicast_spec; @@ -629,6 +628,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2], hsr->redbox = true; ether_addr_copy(hsr->macaddress_redbox, interlink->dev_addr); + timer_setup(&hsr->prune_proxy_timer, hsr_prune_proxy_nodes, 0); mod_timer(&hsr->prune_proxy_timer, jiffies + msecs_to_jiffies(PRUNE_PROXY_PERIOD)); } diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c index 898f18c6da53..c1bd1e6eb955 100644 --- a/net/hsr/hsr_netlink.c +++ b/net/hsr/hsr_netlink.c @@ -129,8 +129,9 @@ static void hsr_dellink(struct net_device *dev, struct list_head *head) struct hsr_priv *hsr = netdev_priv(dev); del_timer_sync(&hsr->prune_timer); - del_timer_sync(&hsr->prune_proxy_timer); del_timer_sync(&hsr->announce_timer); + if (hsr->redbox) + timer_delete_sync(&hsr->prune_proxy_timer); hsr_debugfs_term(hsr); hsr_del_ports(hsr);
The timer for removing entries in the ProxyNodeTable shall be only active when the HSR driver works as RedBox (HSR-SAN). Moreover, the obsolete del_timer_sync() is replaced with timer_delete_sync(). This patch improves fix from commit 3c668cef61ad ("net: hsr: init prune_proxy_timer sooner") as the prune node timer shall be setup only when HSR RedBox is supported in the node. Signed-off-by: Lukasz Majewski <lukma@denx.de> --- net/hsr/hsr_device.c | 2 +- net/hsr/hsr_netlink.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)