All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.