public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code
@ 2026-01-23 17:03 Dmitry Skorodumov
  2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
  To: netdev; +Cc: Dmitry Skorodumov

This is a bit stylish patches: The code to handle ipv4 and ipv6
address change are exactly the same. We don't need separate
functions for them. Just look whether we are called
with ipvlan_addr4_notifier_block or with ipvlan_addr6_notifier_block

The changed functionality is already covered
with existing selftests/net/ipvtap_test.sh

v2:
- Fixed warning about unused vars by specifying __maybe_unused
  for ipv6-related notifier_blocks

Dmitry Skorodumov (3):
  ipvlan: const-specifier for functions that use iaddr
  ipvlan: Common code from v6/v4 validator_event
  ipvlan: common code to handle ipv6/ipv4 address events

 drivers/net/ipvlan/ipvlan.h      |   2 +-
 drivers/net/ipvlan/ipvlan_core.c |   2 +-
 drivers/net/ipvlan/ipvlan_main.c | 188 +++++++++++++------------------
 3 files changed, 83 insertions(+), 109 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr
  2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
  2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, Dmitry Skorodumov, Paolo Abeni,
	Kuniyuki Iwashima, Xiao Liang, Ido Schimmel, Julian Vetter,
	Guillaume Nault, Eric Dumazet, Stanislav Fomichev, linux-kernel
  Cc: Andrew Lunn, David S. Miller

Fix functions that accept "void *iaddr" as param to have
const-specifier.

Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
 drivers/net/ipvlan/ipvlan.h      | 2 +-
 drivers/net/ipvlan/ipvlan_core.c | 2 +-
 drivers/net/ipvlan/ipvlan_main.c | 6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
index 80f84fc87008..235e9218b1bb 100644
--- a/drivers/net/ipvlan/ipvlan.h
+++ b/drivers/net/ipvlan/ipvlan.h
@@ -159,7 +159,7 @@ int ipvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev);
 void ipvlan_ht_addr_add(struct ipvl_dev *ipvlan, struct ipvl_addr *addr);
 struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
 				   const void *iaddr, bool is_v6);
-bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6);
+bool ipvlan_addr_busy(struct ipvl_port *port, const void *iaddr, bool is_v6);
 void ipvlan_ht_addr_del(struct ipvl_addr *addr);
 struct ipvl_addr *ipvlan_addr_lookup(struct ipvl_port *port, void *lyr3h,
 				     int addr_type, bool use_dest);
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index bdb3a46b327c..6d22487010c0 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -118,7 +118,7 @@ struct ipvl_addr *ipvlan_find_addr(const struct ipvl_dev *ipvlan,
 	return NULL;
 }
 
-bool ipvlan_addr_busy(struct ipvl_port *port, void *iaddr, bool is_v6)
+bool ipvlan_addr_busy(struct ipvl_port *port, const void *iaddr, bool is_v6)
 {
 	struct ipvl_dev *ipvlan;
 	bool ret = false;
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index baccdad695fd..cf8c1ea78f4b 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -814,7 +814,8 @@ static int ipvlan_device_event(struct notifier_block *unused,
 }
 
 /* the caller must held the addrs lock */
-static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
+static int ipvlan_add_addr(struct ipvl_dev *ipvlan, const void *iaddr,
+			   bool is_v6)
 {
 	struct ipvl_addr *addr;
 
@@ -846,7 +847,8 @@ static int ipvlan_add_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
 	return 0;
 }
 
-static void ipvlan_del_addr(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6)
+static void ipvlan_del_addr(struct ipvl_dev *ipvlan, const void *iaddr,
+			    bool is_v6)
 {
 	struct ipvl_addr *addr;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
  2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
  2026-01-26 18:01   ` Kuniyuki Iwashima
  2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
  2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman
  3 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, Dmitry Skorodumov, Stanislav Fomichev,
	Xiao Liang, Kuniyuki Iwashima, linux-kernel
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni

Extract commond code for ipvlan_addr4_validator_event()/
ipvlan_addr6_validator_event() to own function.

Get rid of separate functions for xxx_validator_event()
and check whether we are called for ipv4 or ipv6 by
looking at "notifier_block *nblock" argument

Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
v2:
  - Fixed warning about unused by specifying __maybe_unused
    for ipvlan_addr6_vtor_notifier_block (used only with CONFIG_IPV6)

 drivers/net/ipvlan/ipvlan_main.c | 110 +++++++++++++++----------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index cf8c1ea78f4b..d5b55876d340 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -878,6 +878,33 @@ static bool ipvlan_is_valid_dev(const struct net_device *dev)
 	return true;
 }
 
+static int ipvlan_addr_validator_event(struct net_device *dev,
+				       unsigned long event,
+				       struct netlink_ext_ack *extack,
+				       const void *iaddr,
+				       bool is_v6)
+{
+	struct ipvl_dev *ipvlan = netdev_priv(dev);
+	int ret = NOTIFY_OK;
+
+	if (!ipvlan_is_valid_dev(dev))
+		return NOTIFY_DONE;
+
+	switch (event) {
+	case NETDEV_UP:
+		spin_lock_bh(&ipvlan->port->addrs_lock);
+		if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
+			NL_SET_ERR_MSG(extack,
+				       "Address already assigned to an ipvlan device");
+			ret = notifier_from_errno(-EADDRINUSE);
+		}
+		spin_unlock_bh(&ipvlan->port->addrs_lock);
+		break;
+	}
+
+	return ret;
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
 {
@@ -922,32 +949,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
 
 	return NOTIFY_OK;
 }
-
-static int ipvlan_addr6_validator_event(struct notifier_block *unused,
-					unsigned long event, void *ptr)
-{
-	struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
-	struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
-	struct ipvl_dev *ipvlan = netdev_priv(dev);
-	int ret = NOTIFY_OK;
-
-	if (!ipvlan_is_valid_dev(dev))
-		return NOTIFY_DONE;
-
-	switch (event) {
-	case NETDEV_UP:
-		spin_lock_bh(&ipvlan->port->addrs_lock);
-		if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
-			NL_SET_ERR_MSG(i6vi->extack,
-				       "Address already assigned to an ipvlan device");
-			ret = notifier_from_errno(-EADDRINUSE);
-		}
-		spin_unlock_bh(&ipvlan->port->addrs_lock);
-		break;
-	}
-
-	return ret;
-}
 #endif
 
 static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
@@ -997,38 +998,15 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
 	return NOTIFY_OK;
 }
 
-static int ipvlan_addr4_validator_event(struct notifier_block *unused,
-					unsigned long event, void *ptr)
-{
-	struct in_validator_info *ivi = (struct in_validator_info *)ptr;
-	struct net_device *dev = (struct net_device *)ivi->ivi_dev->dev;
-	struct ipvl_dev *ipvlan = netdev_priv(dev);
-	int ret = NOTIFY_OK;
-
-	if (!ipvlan_is_valid_dev(dev))
-		return NOTIFY_DONE;
-
-	switch (event) {
-	case NETDEV_UP:
-		spin_lock_bh(&ipvlan->port->addrs_lock);
-		if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
-			NL_SET_ERR_MSG(ivi->extack,
-				       "Address already assigned to an ipvlan device");
-			ret = notifier_from_errno(-EADDRINUSE);
-		}
-		spin_unlock_bh(&ipvlan->port->addrs_lock);
-		break;
-	}
-
-	return ret;
-}
+static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
+					  unsigned long event, void *ptr);
 
 static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
 	.notifier_call = ipvlan_addr4_event,
 };
 
 static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
-	.notifier_call = ipvlan_addr4_validator_event,
+	.notifier_call = ipvlan_addr_validator_event_cb,
 };
 
 static struct notifier_block ipvlan_notifier_block __read_mostly = {
@@ -1039,11 +1017,33 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
 static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
 	.notifier_call = ipvlan_addr6_event,
 };
+#endif
 
-static struct notifier_block ipvlan_addr6_vtor_notifier_block __read_mostly = {
-	.notifier_call = ipvlan_addr6_validator_event,
+static struct notifier_block
+ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {
+	.notifier_call = ipvlan_addr_validator_event_cb,
 };
-#endif
+
+static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
+					  unsigned long event, void *ptr)
+{
+	struct in6_validator_info *i6vi;
+	struct net_device *dev;
+
+	if (nblock == &ipvlan_addr4_vtor_notifier_block) {
+		struct in_validator_info *ivi;
+
+		ivi = (struct in_validator_info *)ptr;
+		dev = ivi->ivi_dev->dev;
+		return ipvlan_addr_validator_event(dev, event, ivi->extack,
+						   &ivi->ivi_addr, false);
+	}
+
+	i6vi = (struct in6_validator_info *)ptr;
+	dev = i6vi->i6vi_dev->dev;
+	return ipvlan_addr_validator_event(dev, event, i6vi->extack,
+					   &i6vi->i6vi_addr, true);
+}
 
 static int __init ipvlan_init_module(void)
 {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events
  2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
  2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
  2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
@ 2026-01-23 17:03 ` Dmitry Skorodumov
  2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman
  3 siblings, 0 replies; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-23 17:03 UTC (permalink / raw)
  To: netdev, Jakub Kicinski, Dmitry Skorodumov, Kuniyuki Iwashima,
	Stanislav Fomichev, Xiao Liang, linux-kernel
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni

Both IPv4 and IPv6 addr-event functions are very similar. Refactor
to use common funcitons.

Get rid of separate functions for ipvlan_addrX_event()
and check whether we are called for ipv4 or ipv6 by
looking at "notifier_block *nblock" argument

Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
---
v2:
  - Fixed warning about unused by specifying __maybe_unused
    for ipvlan_addr6_notifier_block (used only with CONFIG_IPV6)

 drivers/net/ipvlan/ipvlan_main.c | 114 ++++++++++++-------------------
 1 file changed, 43 insertions(+), 71 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index d5b55876d340..03d0f23dbe33 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -905,93 +905,45 @@ static int ipvlan_addr_validator_event(struct net_device *dev,
 	return ret;
 }
 
-#if IS_ENABLED(CONFIG_IPV6)
-static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
+static int ipvlan_add_addr_event(struct ipvl_dev *ipvlan, const void *iaddr,
+				 bool is_v6)
 {
 	int ret = -EINVAL;
 
 	spin_lock_bh(&ipvlan->port->addrs_lock);
-	if (ipvlan_addr_busy(ipvlan->port, ip6_addr, true))
-		netif_err(ipvlan, ifup, ipvlan->dev,
-			  "Failed to add IPv6=%pI6c addr for %s intf\n",
-			  ip6_addr, ipvlan->dev->name);
-	else
-		ret = ipvlan_add_addr(ipvlan, ip6_addr, true);
-	spin_unlock_bh(&ipvlan->port->addrs_lock);
-	return ret;
-}
-
-static void ipvlan_del_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
-{
-	return ipvlan_del_addr(ipvlan, ip6_addr, true);
-}
-
-static int ipvlan_addr6_event(struct notifier_block *unused,
-			      unsigned long event, void *ptr)
-{
-	struct inet6_ifaddr *if6 = (struct inet6_ifaddr *)ptr;
-	struct net_device *dev = (struct net_device *)if6->idev->dev;
-	struct ipvl_dev *ipvlan = netdev_priv(dev);
-
-	if (!ipvlan_is_valid_dev(dev))
-		return NOTIFY_DONE;
-
-	switch (event) {
-	case NETDEV_UP:
-		if (ipvlan_add_addr6(ipvlan, &if6->addr))
-			return NOTIFY_BAD;
-		break;
-
-	case NETDEV_DOWN:
-		ipvlan_del_addr6(ipvlan, &if6->addr);
-		break;
+	if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
+		if (is_v6) {
+			netif_err(ipvlan, ifup, ipvlan->dev,
+				  "Failed to add IPv6=%pI6c addr on %s intf\n",
+				  iaddr, ipvlan->dev->name);
+		} else {
+			netif_err(ipvlan, ifup, ipvlan->dev,
+				  "Failed to add IPv4=%pI4 on %s intf.\n",
+				  iaddr, ipvlan->dev->name);
+		}
+	} else {
+		ret = ipvlan_add_addr(ipvlan, iaddr, is_v6);
 	}
-
-	return NOTIFY_OK;
-}
-#endif
-
-static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
-{
-	int ret = -EINVAL;
-
-	spin_lock_bh(&ipvlan->port->addrs_lock);
-	if (ipvlan_addr_busy(ipvlan->port, ip4_addr, false))
-		netif_err(ipvlan, ifup, ipvlan->dev,
-			  "Failed to add IPv4=%pI4 on %s intf.\n",
-			  ip4_addr, ipvlan->dev->name);
-	else
-		ret = ipvlan_add_addr(ipvlan, ip4_addr, false);
 	spin_unlock_bh(&ipvlan->port->addrs_lock);
 	return ret;
 }
 
-static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
-{
-	return ipvlan_del_addr(ipvlan, ip4_addr, false);
-}
-
-static int ipvlan_addr4_event(struct notifier_block *unused,
-			      unsigned long event, void *ptr)
+static int ipvlan_addr_event(struct net_device *dev, unsigned long event,
+			     const void *iaddr, bool is_v6)
 {
-	struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
-	struct net_device *dev = (struct net_device *)if4->ifa_dev->dev;
 	struct ipvl_dev *ipvlan = netdev_priv(dev);
-	struct in_addr ip4_addr;
 
 	if (!ipvlan_is_valid_dev(dev))
 		return NOTIFY_DONE;
 
 	switch (event) {
 	case NETDEV_UP:
-		ip4_addr.s_addr = if4->ifa_address;
-		if (ipvlan_add_addr4(ipvlan, &ip4_addr))
+		if (ipvlan_add_addr_event(ipvlan, iaddr, is_v6))
 			return NOTIFY_BAD;
 		break;
 
 	case NETDEV_DOWN:
-		ip4_addr.s_addr = if4->ifa_address;
-		ipvlan_del_addr4(ipvlan, &ip4_addr);
+		ipvlan_del_addr(ipvlan, iaddr, is_v6);
 		break;
 	}
 
@@ -1001,8 +953,11 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
 static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
 					  unsigned long event, void *ptr);
 
+static int ipvlan_addr_event_cb(struct notifier_block *unused,
+				unsigned long event, void *ptr);
+
 static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
-	.notifier_call = ipvlan_addr4_event,
+	.notifier_call = ipvlan_addr_event_cb,
 };
 
 static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
@@ -1013,11 +968,10 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
 	.notifier_call = ipvlan_device_event,
 };
 
-#if IS_ENABLED(CONFIG_IPV6)
-static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
-	.notifier_call = ipvlan_addr6_event,
+static struct notifier_block
+ipvlan_addr6_notifier_block __read_mostly __maybe_unused = {
+	.notifier_call = ipvlan_addr_event_cb,
 };
-#endif
 
 static struct notifier_block
 ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {
@@ -1045,6 +999,24 @@ static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
 					   &i6vi->i6vi_addr, true);
 }
 
+static int ipvlan_addr_event_cb(struct notifier_block *nblock,
+				unsigned long event, void *ptr)
+{
+	struct inet6_ifaddr *if6;
+	struct net_device *dev;
+
+	if (nblock == &ipvlan_addr4_notifier_block) {
+		struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
+
+		dev = if4->ifa_dev->dev;
+		return ipvlan_addr_event(dev, event, &if4->ifa_address, false);
+	}
+
+	if6 = (struct inet6_ifaddr *)ptr;
+	dev = if6->idev->dev;
+	return ipvlan_addr_event(dev, event, &if6->addr, true);
+}
+
 static int __init ipvlan_init_module(void)
 {
 	int err;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code
  2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
                   ` (2 preceding siblings ...)
  2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
@ 2026-01-26 15:06 ` Simon Horman
  3 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2026-01-26 15:06 UTC (permalink / raw)
  To: Dmitry Skorodumov; +Cc: netdev, Dmitry Skorodumov

On Fri, Jan 23, 2026 at 08:03:53PM +0300, Dmitry Skorodumov wrote:
> This is a bit stylish patches: The code to handle ipv4 and ipv6
> address change are exactly the same. We don't need separate
> functions for them. Just look whether we are called
> with ipvlan_addr4_notifier_block or with ipvlan_addr6_notifier_block
> 
> The changed functionality is already covered
> with existing selftests/net/ipvtap_test.sh

Hi Dmitry,

I'm somewhat ambivalent towards the trade-off between
reducing code duplication, and requiring run-time demuxing
between ipv4 and ipv4 code paths and the moderate increase in
code complexity that entails.

But assuming these paths are not performance sensitive I guess
that reducing duplication has the upper hand.

Reviewed-by: Simon Horman <horms@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
@ 2026-01-26 18:01   ` Kuniyuki Iwashima
  2026-01-27  5:56     ` Dmitry Skorodumov
  0 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-26 18:01 UTC (permalink / raw)
  To: Dmitry Skorodumov
  Cc: netdev, Jakub Kicinski, Dmitry Skorodumov, Stanislav Fomichev,
	Xiao Liang, linux-kernel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:
>
> Extract commond code for ipvlan_addr4_validator_event()/
> ipvlan_addr6_validator_event() to own function.
>
> Get rid of separate functions for xxx_validator_event()
> and check whether we are called for ipv4 or ipv6 by
> looking at "notifier_block *nblock" argument
>
> Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@huawei.com>
> ---
> v2:
>   - Fixed warning about unused by specifying __maybe_unused
>     for ipvlan_addr6_vtor_notifier_block (used only with CONFIG_IPV6)
>
>  drivers/net/ipvlan/ipvlan_main.c | 110 +++++++++++++++----------------
>  1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index cf8c1ea78f4b..d5b55876d340 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -878,6 +878,33 @@ static bool ipvlan_is_valid_dev(const struct net_device *dev)
>         return true;
>  }
>
> +static int ipvlan_addr_validator_event(struct net_device *dev,
> +                                      unsigned long event,
> +                                      struct netlink_ext_ack *extack,
> +                                      const void *iaddr,
> +                                      bool is_v6)
> +{
> +       struct ipvl_dev *ipvlan = netdev_priv(dev);
> +       int ret = NOTIFY_OK;
> +
> +       if (!ipvlan_is_valid_dev(dev))
> +               return NOTIFY_DONE;
> +
> +       switch (event) {
> +       case NETDEV_UP:
> +               spin_lock_bh(&ipvlan->port->addrs_lock);
> +               if (ipvlan_addr_busy(ipvlan->port, iaddr, is_v6)) {
> +                       NL_SET_ERR_MSG(extack,
> +                                      "Address already assigned to an ipvlan device");
> +                       ret = notifier_from_errno(-EADDRINUSE);
> +               }
> +               spin_unlock_bh(&ipvlan->port->addrs_lock);
> +               break;
> +       }
> +
> +       return ret;
> +}
> +
>  #if IS_ENABLED(CONFIG_IPV6)
>  static int ipvlan_add_addr6(struct ipvl_dev *ipvlan, struct in6_addr *ip6_addr)
>  {
> @@ -922,32 +949,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
>
>         return NOTIFY_OK;
>  }
> -
> -static int ipvlan_addr6_validator_event(struct notifier_block *unused,
> -                                       unsigned long event, void *ptr)
> -{
> -       struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
> -       struct net_device *dev = (struct net_device *)i6vi->i6vi_dev->dev;
> -       struct ipvl_dev *ipvlan = netdev_priv(dev);
> -       int ret = NOTIFY_OK;
> -
> -       if (!ipvlan_is_valid_dev(dev))
> -               return NOTIFY_DONE;
> -
> -       switch (event) {
> -       case NETDEV_UP:
> -               spin_lock_bh(&ipvlan->port->addrs_lock);
> -               if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
> -                       NL_SET_ERR_MSG(i6vi->extack,
> -                                      "Address already assigned to an ipvlan device");
> -                       ret = notifier_from_errno(-EADDRINUSE);
> -               }
> -               spin_unlock_bh(&ipvlan->port->addrs_lock);
> -               break;
> -       }
> -
> -       return ret;
> -}
>  #endif
>
>  static int ipvlan_add_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
> @@ -997,38 +998,15 @@ static int ipvlan_addr4_event(struct notifier_block *unused,
>         return NOTIFY_OK;
>  }
>
> -static int ipvlan_addr4_validator_event(struct notifier_block *unused,
> -                                       unsigned long event, void *ptr)
> -{
> -       struct in_validator_info *ivi = (struct in_validator_info *)ptr;
> -       struct net_device *dev = (struct net_device *)ivi->ivi_dev->dev;
> -       struct ipvl_dev *ipvlan = netdev_priv(dev);
> -       int ret = NOTIFY_OK;
> -
> -       if (!ipvlan_is_valid_dev(dev))
> -               return NOTIFY_DONE;
> -
> -       switch (event) {
> -       case NETDEV_UP:
> -               spin_lock_bh(&ipvlan->port->addrs_lock);
> -               if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
> -                       NL_SET_ERR_MSG(ivi->extack,
> -                                      "Address already assigned to an ipvlan device");
> -                       ret = notifier_from_errno(-EADDRINUSE);
> -               }
> -               spin_unlock_bh(&ipvlan->port->addrs_lock);
> -               break;
> -       }
> -
> -       return ret;
> -}
> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> +                                         unsigned long event, void *ptr);
>
>  static struct notifier_block ipvlan_addr4_notifier_block __read_mostly = {
>         .notifier_call = ipvlan_addr4_event,
>  };
>
>  static struct notifier_block ipvlan_addr4_vtor_notifier_block __read_mostly = {
> -       .notifier_call = ipvlan_addr4_validator_event,
> +       .notifier_call = ipvlan_addr_validator_event_cb,
>  };
>
>  static struct notifier_block ipvlan_notifier_block __read_mostly = {
> @@ -1039,11 +1017,33 @@ static struct notifier_block ipvlan_notifier_block __read_mostly = {
>  static struct notifier_block ipvlan_addr6_notifier_block __read_mostly = {
>         .notifier_call = ipvlan_addr6_event,
>  };
> +#endif
>
> -static struct notifier_block ipvlan_addr6_vtor_notifier_block __read_mostly = {
> -       .notifier_call = ipvlan_addr6_validator_event,
> +static struct notifier_block
> +ipvlan_addr6_vtor_notifier_block __read_mostly __maybe_unused = {

I think #if IS_ENABLED(CONFIG_IPV6) is preferable to
__mabe_unused.


> +       .notifier_call = ipvlan_addr_validator_event_cb,
>  };
> -#endif
> +
> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> +                                         unsigned long event, void *ptr)
> +{
> +       struct in6_validator_info *i6vi;
> +       struct net_device *dev;
> +
> +       if (nblock == &ipvlan_addr4_vtor_notifier_block) {

If you check this against the v6 one instead, the if block
and the i6vi definition can be guarded with
#if IS_ENABLED(CONFIG_IPV6)


> +               struct in_validator_info *ivi;
> +
> +               ivi = (struct in_validator_info *)ptr;
> +               dev = ivi->ivi_dev->dev;
> +               return ipvlan_addr_validator_event(dev, event, ivi->extack,
> +                                                  &ivi->ivi_addr, false);
> +       }
> +
> +       i6vi = (struct in6_validator_info *)ptr;
> +       dev = i6vi->i6vi_dev->dev;
> +       return ipvlan_addr_validator_event(dev, event, i6vi->extack,
> +                                          &i6vi->i6vi_addr, true);
> +}
>
>  static int __init ipvlan_init_module(void)
>  {
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-26 18:01   ` Kuniyuki Iwashima
@ 2026-01-27  5:56     ` Dmitry Skorodumov
  2026-01-27  7:37       ` Eric Dumazet
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-27  5:56 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Dmitry Skorodumov
  Cc: netdev, Jakub Kicinski, Stanislav Fomichev, Xiao Liang,
	linux-kernel, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni

On 1/26/2026 9:01 PM, Kuniyuki Iwashima wrote:
> On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:

>> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
>> +                                         unsigned long event, void *ptr)
>> +{
>> +       struct in6_validator_info *i6vi;
>> +       struct net_device *dev;
>> +
>> +       if (nblock == &ipvlan_addr4_vtor_notifier_block) {
> 
> If you check this against the v6 one instead, the if block
> and the i6vi definition can be guarded with
> #if IS_ENABLED(CONFIG_IPV6)

Hm... Few months ago, I had a conversation here that convinced me: avoid 
#ifdef whenever possible. They overburden code and reduce readability.

It even might be a good idea to replace wherever possible preprocessor 
checks with runtime checks. Use
if (IS_ENABLED(CONFIG_IPV6)) { ... }

instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif

--

I'm ok with both approaches (though i tend to like runtime checks) - but 
unsure what is a common practice in bpf-net

Dmitry


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-27  5:56     ` Dmitry Skorodumov
@ 2026-01-27  7:37       ` Eric Dumazet
  2026-01-27  7:54         ` Dmitry Skorodumov
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2026-01-27  7:37 UTC (permalink / raw)
  To: Dmitry Skorodumov
  Cc: Kuniyuki Iwashima, Dmitry Skorodumov, netdev, Jakub Kicinski,
	Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
	David S. Miller, Paolo Abeni

On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
<skorodumov.dmitry@huawei.com> wrote:
>
> On 1/26/2026 9:01 PM, Kuniyuki Iwashima wrote:
> > On Fri, Jan 23, 2026 at 9:04 AM Dmitry Skorodumov <dskr99@gmail.com> wrote:
>
> >> +static int ipvlan_addr_validator_event_cb(struct notifier_block *nblock,
> >> +                                         unsigned long event, void *ptr)
> >> +{
> >> +       struct in6_validator_info *i6vi;
> >> +       struct net_device *dev;
> >> +
> >> +       if (nblock == &ipvlan_addr4_vtor_notifier_block) {
> >
> > If you check this against the v6 one instead, the if block
> > and the i6vi definition can be guarded with
> > #if IS_ENABLED(CONFIG_IPV6)
>
> Hm... Few months ago, I had a conversation here that convinced me: avoid
> #ifdef whenever possible. They overburden code and reduce readability.
>
> It even might be a good idea to replace wherever possible preprocessor
> checks with runtime checks. Use
> if (IS_ENABLED(CONFIG_IPV6)) { ... }
>
> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
>
> --
>
> I'm ok with both approaches (though i tend to like runtime checks) - but
> unsure what is a common practice in bpf-net

What you call runtime checks is in reality the same : C compiler is
able to optimize constant boolean expressions

if (0) {
  code_0;
}

if (0 &&  anything) {
  code_0_1;
}

-> Compiler will completely remove all of this.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-27  7:37       ` Eric Dumazet
@ 2026-01-27  7:54         ` Dmitry Skorodumov
  2026-01-27  8:11           ` Kuniyuki Iwashima
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Skorodumov @ 2026-01-27  7:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Kuniyuki Iwashima, Dmitry Skorodumov, netdev, Jakub Kicinski,
	Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
	David S. Miller, Paolo Abeni

On 1/27/2026 10:37 AM, Eric Dumazet wrote:
> On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
> <skorodumov.dmitry@huawei.com> wrote:
>> It even might be a good idea to replace wherever possible preprocessor
>> checks with runtime checks. Use
>> if (IS_ENABLED(CONFIG_IPV6)) { ... }
>>
>> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
>>
>> --
>>
>> I'm ok with both approaches (though i tend to like runtime checks) - but
>> unsure what is a common practice in bpf-net
> 
> What you call runtime checks is in reality the same : C compiler is
> able to optimize constant boolean expressions

Should I fix patches to be something like

if (nblock == &ipvlan_addr4_vtor_notifier_block) {
} else if (IS_ENABLED(CONFIG_IPV6)) {
   ...
}

but still with __maybe_unused for ipvlan_addr6_vtor_notifier_block ?

I see that this __maybe_unused is used quite often in linux-kernel in 
similar way

Dmitry


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event
  2026-01-27  7:54         ` Dmitry Skorodumov
@ 2026-01-27  8:11           ` Kuniyuki Iwashima
  0 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2026-01-27  8:11 UTC (permalink / raw)
  To: Dmitry Skorodumov
  Cc: Eric Dumazet, Dmitry Skorodumov, netdev, Jakub Kicinski,
	Stanislav Fomichev, Xiao Liang, linux-kernel, Andrew Lunn,
	David S. Miller, Paolo Abeni

On Mon, Jan 26, 2026 at 11:54 PM Dmitry Skorodumov
<skorodumov.dmitry@huawei.com> wrote:
>
> On 1/27/2026 10:37 AM, Eric Dumazet wrote:
> > On Tue, Jan 27, 2026 at 6:56 AM Dmitry Skorodumov
> > <skorodumov.dmitry@huawei.com> wrote:
> >> It even might be a good idea to replace wherever possible preprocessor
> >> checks with runtime checks. Use
> >> if (IS_ENABLED(CONFIG_IPV6)) { ... }
> >>
> >> instead of #if IS_ENABLED(CONFIG_IPV6) ... #endif
> >>
> >> --
> >>
> >> I'm ok with both approaches (though i tend to like runtime checks) - but
> >> unsure what is a common practice in bpf-net
> >
> > What you call runtime checks is in reality the same : C compiler is
> > able to optimize constant boolean expressions
>
> Should I fix patches to be something like
>
> if (nblock == &ipvlan_addr4_vtor_notifier_block) {
> } else if (IS_ENABLED(CONFIG_IPV6)) {
>    ...
> }
>
> but still with __maybe_unused for ipvlan_addr6_vtor_notifier_block ?

If IS_ENABLED() or #if is used properly, __maybe_unused
should be unnecessary.  Actually the patch moved
ipvlan_addr6_vtor_notifier_block out of the existing #if guard.

Also, ipvlan_main.c already has such #if guards.

>
> I see that this __maybe_unused is used quite often in linux-kernel in
> similar way

but not that under net/.

$ grep -rnI __maybe_unused | wc -l
4827
$ grep -rnI __maybe_unused net | wc -l
33

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-01-27  8:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-23 17:03 [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 1/3] ipvlan: const-specifier for functions that use iaddr Dmitry Skorodumov
2026-01-23 17:03 ` [PATCH v2 net-next 2/3] ipvlan: Common code from v6/v4 validator_event Dmitry Skorodumov
2026-01-26 18:01   ` Kuniyuki Iwashima
2026-01-27  5:56     ` Dmitry Skorodumov
2026-01-27  7:37       ` Eric Dumazet
2026-01-27  7:54         ` Dmitry Skorodumov
2026-01-27  8:11           ` Kuniyuki Iwashima
2026-01-23 17:03 ` [PATCH v2 net-next 3/3] ipvlan: common code to handle ipv6/ipv4 address events Dmitry Skorodumov
2026-01-26 15:06 ` [PATCH v2 net-next 0/3] ipvlan: Deduplicate ipv4/ipv6 addr_validator_event code Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox