[PATCH net-next 0/5] Code movement to br_switchdev.c

[PATCH net-next 0/5] Code movement to br_switchdev.c

[Vladimir Oltean]

This is one more refactoring patch set for the Linux bridge, where more
logic that is specific to switchdev is moved into br_switchdev.c, which
is compiled out when CONFIG_NET_SWITCHDEV is disabled.

Vladimir Oltean (5):
  net: bridge: provide shim definition for br_vlan_flags
  net: bridge: move br_vlan_replay to br_switchdev.c
  net: bridge: split out the switchdev portion of br_mdb_notify
  net: bridge: mdb: move all switchdev logic to br_switchdev.c
  net: bridge: switchdev: consistent function naming

 net/bridge/br_mdb.c       | 238 +-----------------------
 net/bridge/br_private.h   |  28 ++-
 net/bridge/br_switchdev.c | 371 ++++++++++++++++++++++++++++++++++++--
 net/bridge/br_vlan.c      |  84 ---------
 4 files changed, 372 insertions(+), 349 deletions(-)

-- 
2.25.1

[PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Vladimir Oltean]

br_vlan_replay() needs this, and we're preparing to move it to
br_switchdev.c, which will be compiled regardless of whether or not
CONFIG_BRIDGE_VLAN_FILTERING is enabled.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_private.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 3c9327628060..cc31c3fe1e02 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
 	return true;
 }
 
+static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
+{
+	return 0;
+}
+
 static inline int br_vlan_replay(struct net_device *br_dev,
 				 struct net_device *dev, const void *ctx,
 				 bool adding, struct notifier_block *nb,
-- 
2.25.1

Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Nikolay Aleksandrov]

On 27/10/2021 19:21, Vladimir Oltean wrote:
> br_vlan_replay() needs this, and we're preparing to move it to
> br_switchdev.c, which will be compiled regardless of whether or not
> CONFIG_BRIDGE_VLAN_FILTERING is enabled.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_private.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 3c9327628060..cc31c3fe1e02 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
>  	return true;
>  }
>  
> +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
> +{
> +	return 0;
> +}
> +
>  static inline int br_vlan_replay(struct net_device *br_dev,
>  				 struct net_device *dev, const void *ctx,
>  				 bool adding, struct notifier_block *nb,
> 

hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
I.e. shouldn't this rather be turned into br_vlan_replay's shim?

TBH, I haven't looked into the details just wonder why we would compile all that vlan
code if bridge vlan filtering is not enabled.

Thanks,
 Nik

Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Vladimir Oltean]

On Wed, Oct 27, 2021 at 10:28:12PM +0300, Nikolay Aleksandrov wrote:
> On 27/10/2021 19:21, Vladimir Oltean wrote:
> > br_vlan_replay() needs this, and we're preparing to move it to
> > br_switchdev.c, which will be compiled regardless of whether or not
> > CONFIG_BRIDGE_VLAN_FILTERING is enabled.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  net/bridge/br_private.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index 3c9327628060..cc31c3fe1e02 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
> >  	return true;
> >  }
> >  
> > +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
> > +{
> > +	return 0;
> > +}
> > +
> >  static inline int br_vlan_replay(struct net_device *br_dev,
> >  				 struct net_device *dev, const void *ctx,
> >  				 bool adding, struct notifier_block *nb,
> > 
> 
> hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
> I.e. shouldn't this rather be turned into br_vlan_replay's shim?
> 
> TBH, I haven't looked into the details just wonder why we would compile all that vlan
> code if bridge vlan filtering is not enabled.

The main reason is that I would like to avoid #ifdef if possible. If you
have a strong opinion otherwise I can follow suit.

Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Nikolay Aleksandrov]

On 27/10/2021 22:45, Vladimir Oltean wrote:
> On Wed, Oct 27, 2021 at 10:28:12PM +0300, Nikolay Aleksandrov wrote:
>> On 27/10/2021 19:21, Vladimir Oltean wrote:
>>> br_vlan_replay() needs this, and we're preparing to move it to
>>> br_switchdev.c, which will be compiled regardless of whether or not
>>> CONFIG_BRIDGE_VLAN_FILTERING is enabled.
>>>
>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>> ---
>>>  net/bridge/br_private.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index 3c9327628060..cc31c3fe1e02 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
>>>  	return true;
>>>  }
>>>  
>>> +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>>  static inline int br_vlan_replay(struct net_device *br_dev,
>>>  				 struct net_device *dev, const void *ctx,
>>>  				 bool adding, struct notifier_block *nb,
>>>
>>
>> hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
>> I.e. shouldn't this rather be turned into br_vlan_replay's shim?
>>
>> TBH, I haven't looked into the details just wonder why we would compile all that vlan
>> code if bridge vlan filtering is not enabled.
> 
> The main reason is that I would like to avoid #ifdef if possible. If you
> have a strong opinion otherwise I can follow suit.
> 

Well, I see that we add ifdefs for IGMP, so I don't see a reason why not
to ifdef out the vlan replay in the same way too.

I don't have a strong preference either way, end result is the same.

Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Vladimir Oltean]

On Wed, Oct 27, 2021 at 10:50:47PM +0300, Nikolay Aleksandrov wrote:
> On 27/10/2021 22:45, Vladimir Oltean wrote:
> > On Wed, Oct 27, 2021 at 10:28:12PM +0300, Nikolay Aleksandrov wrote:
> >> On 27/10/2021 19:21, Vladimir Oltean wrote:
> >>> br_vlan_replay() needs this, and we're preparing to move it to
> >>> br_switchdev.c, which will be compiled regardless of whether or not
> >>> CONFIG_BRIDGE_VLAN_FILTERING is enabled.
> >>>
> >>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >>> ---
> >>>  net/bridge/br_private.h | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>> index 3c9327628060..cc31c3fe1e02 100644
> >>> --- a/net/bridge/br_private.h
> >>> +++ b/net/bridge/br_private.h
> >>> @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
> >>>  	return true;
> >>>  }
> >>>  
> >>> +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
> >>> +{
> >>> +	return 0;
> >>> +}
> >>> +
> >>>  static inline int br_vlan_replay(struct net_device *br_dev,
> >>>  				 struct net_device *dev, const void *ctx,
> >>>  				 bool adding, struct notifier_block *nb,
> >>>
> >>
> >> hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
> >> I.e. shouldn't this rather be turned into br_vlan_replay's shim?
> >>
> >> TBH, I haven't looked into the details just wonder why we would compile all that vlan
> >> code if bridge vlan filtering is not enabled.
> > 
> > The main reason is that I would like to avoid #ifdef if possible. If you
> > have a strong opinion otherwise I can follow suit.
> > 
> 
> Well, I see that we add ifdefs for IGMP, so I don't see a reason why not
> to ifdef out the vlan replay in the same way too.
> 
> I don't have a strong preference either way, end result is the same.

Since the caller and the callee are in the same C file, shimming out is
not as clean as providing a static inline function definition with an
empty body, and if I could avoid doing what I did for

br_mdb_replay()
{
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
	<some other variables>;
	int err;

	err = <body>;

	if (err)
		return err;
#endif

	return 0;
}

I'd do it. For br_vlan_replay() I could avoid it, so I left it at that.

Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan_flags

[Nikolay Aleksandrov]

On 27/10/2021 22:54, Vladimir Oltean wrote:
> On Wed, Oct 27, 2021 at 10:50:47PM +0300, Nikolay Aleksandrov wrote:
>> On 27/10/2021 22:45, Vladimir Oltean wrote:
>>> On Wed, Oct 27, 2021 at 10:28:12PM +0300, Nikolay Aleksandrov wrote:
>>>> On 27/10/2021 19:21, Vladimir Oltean wrote:
>>>>> br_vlan_replay() needs this, and we're preparing to move it to
>>>>> br_switchdev.c, which will be compiled regardless of whether or not
>>>>> CONFIG_BRIDGE_VLAN_FILTERING is enabled.
>>>>>
>>>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>>>>> ---
>>>>>  net/bridge/br_private.h | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>> index 3c9327628060..cc31c3fe1e02 100644
>>>>> --- a/net/bridge/br_private.h
>>>>> +++ b/net/bridge/br_private.h
>>>>> @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
>>>>>  	return true;
>>>>>  }
>>>>>  
>>>>> +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
>>>>> +{
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>>  static inline int br_vlan_replay(struct net_device *br_dev,
>>>>>  				 struct net_device *dev, const void *ctx,
>>>>>  				 bool adding, struct notifier_block *nb,
>>>>>
>>>>
>>>> hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
>>>> I.e. shouldn't this rather be turned into br_vlan_replay's shim?
>>>>
>>>> TBH, I haven't looked into the details just wonder why we would compile all that vlan
>>>> code if bridge vlan filtering is not enabled.
>>>
>>> The main reason is that I would like to avoid #ifdef if possible. If you
>>> have a strong opinion otherwise I can follow suit.
>>>
>>
>> Well, I see that we add ifdefs for IGMP, so I don't see a reason why not
>> to ifdef out the vlan replay in the same way too.
>>
>> I don't have a strong preference either way, end result is the same.
> 
> Since the caller and the callee are in the same C file, shimming out is
> not as clean as providing a static inline function definition with an
> empty body, and if I could avoid doing what I did for
> 
> br_mdb_replay()
> {
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> 	<some other variables>;
> 	int err;
> 
> 	err = <body>;
> 
> 	if (err)
> 		return err;
> #endif
> 
> 	return 0;
> }
> 
> I'd do it. For br_vlan_replay() I could avoid it, so I left it at that.
> 

that's ok,
Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>

[PATCH net-next 2/5] net: bridge: move br_vlan_replay to br_switchdev.c

[Vladimir Oltean]

br_vlan_replay() is relevant only if CONFIG_NET_SWITCHDEV is enabled, so
move it to br_switchdev.c.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_private.h   | 10 -----
 net/bridge/br_switchdev.c | 85 +++++++++++++++++++++++++++++++++++++++
 net/bridge/br_vlan.c      | 84 --------------------------------------
 3 files changed, 85 insertions(+), 94 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index cc31c3fe1e02..b16c83e10356 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1459,9 +1459,6 @@ void br_vlan_notify(const struct net_bridge *br,
 		    const struct net_bridge_port *p,
 		    u16 vid, u16 vid_range,
 		    int cmd);
-int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
-		   const void *ctx, bool adding, struct notifier_block *nb,
-		   struct netlink_ext_ack *extack);
 bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
 			     const struct net_bridge_vlan *range_end);
 
@@ -1713,13 +1710,6 @@ static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
 	return 0;
 }
 
-static inline int br_vlan_replay(struct net_device *br_dev,
-				 struct net_device *dev, const void *ctx,
-				 bool adding, struct notifier_block *nb,
-				 struct netlink_ext_ack *extack)
-{
-	return -EOPNOTSUPP;
-}
 #endif
 
 /* br_vlan_options.c */
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 2fbe881cdfe2..d773d819a867 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -327,6 +327,91 @@ static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
 	return err;
 }
 
+static int br_vlan_replay_one(struct notifier_block *nb,
+			      struct net_device *dev,
+			      struct switchdev_obj_port_vlan *vlan,
+			      const void *ctx, unsigned long action,
+			      struct netlink_ext_ack *extack)
+{
+	struct switchdev_notifier_port_obj_info obj_info = {
+		.info = {
+			.dev = dev,
+			.extack = extack,
+			.ctx = ctx,
+		},
+		.obj = &vlan->obj,
+	};
+	int err;
+
+	err = nb->notifier_call(nb, action, &obj_info);
+	return notifier_to_errno(err);
+}
+
+static int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
+			  const void *ctx, bool adding,
+			  struct notifier_block *nb,
+			  struct netlink_ext_ack *extack)
+{
+	struct net_bridge_vlan_group *vg;
+	struct net_bridge_vlan *v;
+	struct net_bridge_port *p;
+	struct net_bridge *br;
+	unsigned long action;
+	int err = 0;
+	u16 pvid;
+
+	ASSERT_RTNL();
+
+	if (!nb)
+		return 0;
+
+	if (!netif_is_bridge_master(br_dev))
+		return -EINVAL;
+
+	if (!netif_is_bridge_master(dev) && !netif_is_bridge_port(dev))
+		return -EINVAL;
+
+	if (netif_is_bridge_master(dev)) {
+		br = netdev_priv(dev);
+		vg = br_vlan_group(br);
+		p = NULL;
+	} else {
+		p = br_port_get_rtnl(dev);
+		if (WARN_ON(!p))
+			return -EINVAL;
+		vg = nbp_vlan_group(p);
+		br = p->br;
+	}
+
+	if (!vg)
+		return 0;
+
+	if (adding)
+		action = SWITCHDEV_PORT_OBJ_ADD;
+	else
+		action = SWITCHDEV_PORT_OBJ_DEL;
+
+	pvid = br_get_pvid(vg);
+
+	list_for_each_entry(v, &vg->vlan_list, vlist) {
+		struct switchdev_obj_port_vlan vlan = {
+			.obj.orig_dev = dev,
+			.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+			.flags = br_vlan_flags(v, pvid),
+			.vid = v->vid,
+		};
+
+		if (!br_vlan_should_use(v))
+			continue;
+
+		err = br_vlan_replay_one(nb, dev, &vlan, ctx, action, extack);
+		if (err)
+			return err;
+	}
+
+	return err;
+}
+
 static int nbp_switchdev_sync_objs(struct net_bridge_port *p, const void *ctx,
 				   struct notifier_block *atomic_nb,
 				   struct notifier_block *blocking_nb,
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 57bd6ee72a07..49e105e0a447 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1860,90 +1860,6 @@ void br_vlan_notify(const struct net_bridge *br,
 	kfree_skb(skb);
 }
 
-static int br_vlan_replay_one(struct notifier_block *nb,
-			      struct net_device *dev,
-			      struct switchdev_obj_port_vlan *vlan,
-			      const void *ctx, unsigned long action,
-			      struct netlink_ext_ack *extack)
-{
-	struct switchdev_notifier_port_obj_info obj_info = {
-		.info = {
-			.dev = dev,
-			.extack = extack,
-			.ctx = ctx,
-		},
-		.obj = &vlan->obj,
-	};
-	int err;
-
-	err = nb->notifier_call(nb, action, &obj_info);
-	return notifier_to_errno(err);
-}
-
-int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
-		   const void *ctx, bool adding, struct notifier_block *nb,
-		   struct netlink_ext_ack *extack)
-{
-	struct net_bridge_vlan_group *vg;
-	struct net_bridge_vlan *v;
-	struct net_bridge_port *p;
-	struct net_bridge *br;
-	unsigned long action;
-	int err = 0;
-	u16 pvid;
-
-	ASSERT_RTNL();
-
-	if (!nb)
-		return 0;
-
-	if (!netif_is_bridge_master(br_dev))
-		return -EINVAL;
-
-	if (!netif_is_bridge_master(dev) && !netif_is_bridge_port(dev))
-		return -EINVAL;
-
-	if (netif_is_bridge_master(dev)) {
-		br = netdev_priv(dev);
-		vg = br_vlan_group(br);
-		p = NULL;
-	} else {
-		p = br_port_get_rtnl(dev);
-		if (WARN_ON(!p))
-			return -EINVAL;
-		vg = nbp_vlan_group(p);
-		br = p->br;
-	}
-
-	if (!vg)
-		return 0;
-
-	if (adding)
-		action = SWITCHDEV_PORT_OBJ_ADD;
-	else
-		action = SWITCHDEV_PORT_OBJ_DEL;
-
-	pvid = br_get_pvid(vg);
-
-	list_for_each_entry(v, &vg->vlan_list, vlist) {
-		struct switchdev_obj_port_vlan vlan = {
-			.obj.orig_dev = dev,
-			.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
-			.flags = br_vlan_flags(v, pvid),
-			.vid = v->vid,
-		};
-
-		if (!br_vlan_should_use(v))
-			continue;
-
-		err = br_vlan_replay_one(nb, dev, &vlan, ctx, action, extack);
-		if (err)
-			return err;
-	}
-
-	return err;
-}
-
 /* check if v_curr can enter a range ending in range_end */
 bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
 			     const struct net_bridge_vlan *range_end)
-- 
2.25.1

Re: [PATCH net-next 2/5] net: bridge: move br_vlan_replay to br_switchdev.c

[Nikolay Aleksandrov]

On 27/10/2021 19:21, Vladimir Oltean wrote:
> br_vlan_replay() is relevant only if CONFIG_NET_SWITCHDEV is enabled, so
> move it to br_switchdev.c.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_private.h   | 10 -----
>  net/bridge/br_switchdev.c | 85 +++++++++++++++++++++++++++++++++++++++
>  net/bridge/br_vlan.c      | 84 --------------------------------------
>  3 files changed, 85 insertions(+), 94 deletions(-)

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>

[PATCH net-next 3/5] net: bridge: split out the switchdev portion of br_mdb_notify

[Vladimir Oltean]

Similar to fdb_notify() and br_switchdev_fdb_notify(), split the
switchdev specific logic from br_mdb_notify() into a different function.
This will be moved later in br_switchdev.c.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_mdb.c | 62 +++++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 61ccf46fcc21..9513f0791c3d 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -759,10 +759,10 @@ static void br_mdb_switchdev_host(struct net_device *dev,
 		br_mdb_switchdev_host_port(dev, lower_dev, mp, type);
 }
 
-void br_mdb_notify(struct net_device *dev,
-		   struct net_bridge_mdb_entry *mp,
-		   struct net_bridge_port_group *pg,
-		   int type)
+static void br_switchdev_mdb_notify(struct net_device *dev,
+				    struct net_bridge_mdb_entry *mp,
+				    struct net_bridge_port_group *pg,
+				    int type)
 {
 	struct br_mdb_complete_info *complete_info;
 	struct switchdev_obj_port_mdb mdb = {
@@ -771,33 +771,41 @@ void br_mdb_notify(struct net_device *dev,
 			.flags = SWITCHDEV_F_DEFER,
 		},
 	};
-	struct net *net = dev_net(dev);
-	struct sk_buff *skb;
-	int err = -ENOBUFS;
 
-	if (pg) {
-		br_switchdev_mdb_populate(&mdb, mp);
+	if (!pg)
+		return br_mdb_switchdev_host(dev, mp, type);
 
-		mdb.obj.orig_dev = pg->key.port->dev;
-		switch (type) {
-		case RTM_NEWMDB:
-			complete_info = kmalloc(sizeof(*complete_info), GFP_ATOMIC);
-			if (!complete_info)
-				break;
-			complete_info->port = pg->key.port;
-			complete_info->ip = mp->addr;
-			mdb.obj.complete_priv = complete_info;
-			mdb.obj.complete = br_mdb_complete;
-			if (switchdev_port_obj_add(pg->key.port->dev, &mdb.obj, NULL))
-				kfree(complete_info);
-			break;
-		case RTM_DELMDB:
-			switchdev_port_obj_del(pg->key.port->dev, &mdb.obj);
+	br_switchdev_mdb_populate(&mdb, mp);
+
+	mdb.obj.orig_dev = pg->key.port->dev;
+	switch (type) {
+	case RTM_NEWMDB:
+		complete_info = kmalloc(sizeof(*complete_info), GFP_ATOMIC);
+		if (!complete_info)
 			break;
-		}
-	} else {
-		br_mdb_switchdev_host(dev, mp, type);
+		complete_info->port = pg->key.port;
+		complete_info->ip = mp->addr;
+		mdb.obj.complete_priv = complete_info;
+		mdb.obj.complete = br_mdb_complete;
+		if (switchdev_port_obj_add(pg->key.port->dev, &mdb.obj, NULL))
+			kfree(complete_info);
+		break;
+	case RTM_DELMDB:
+		switchdev_port_obj_del(pg->key.port->dev, &mdb.obj);
+		break;
 	}
+}
+
+void br_mdb_notify(struct net_device *dev,
+		   struct net_bridge_mdb_entry *mp,
+		   struct net_bridge_port_group *pg,
+		   int type)
+{
+	struct net *net = dev_net(dev);
+	struct sk_buff *skb;
+	int err = -ENOBUFS;
+
+	br_switchdev_mdb_notify(dev, mp, pg, type);
 
 	skb = nlmsg_new(rtnl_mdb_nlmsg_size(pg), GFP_ATOMIC);
 	if (!skb)
-- 
2.25.1

Re: [PATCH net-next 3/5] net: bridge: split out the switchdev portion of br_mdb_notify

[Nikolay Aleksandrov]

On 27/10/2021 19:21, Vladimir Oltean wrote:
> Similar to fdb_notify() and br_switchdev_fdb_notify(), split the
> switchdev specific logic from br_mdb_notify() into a different function.
> This will be moved later in br_switchdev.c.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_mdb.c | 62 +++++++++++++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 27 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>

[PATCH net-next 4/5] net: bridge: mdb: move all switchdev logic to br_switchdev.c

[Vladimir Oltean]

The following functions:

br_mdb_complete
br_switchdev_mdb_populate
br_mdb_replay_one
br_mdb_queue_one
br_mdb_replay
br_mdb_switchdev_host_port
br_mdb_switchdev_host
br_switchdev_mdb_notify

are only accessible from code paths where CONFIG_NET_SWITCHDEV is
enabled. So move them to br_switchdev.c, in order for that code to be
compiled out if that config option is disabled.

Note that br_switchdev.c gets build regardless of whether
CONFIG_BRIDGE_IGMP_SNOOPING is enabled or not, whereas br_mdb.c only got
built when CONFIG_BRIDGE_IGMP_SNOOPING was enabled. So to preserve
correct compilation with CONFIG_BRIDGE_IGMP_SNOOPING being disabled, we
must now place an #ifdef around these functions in br_switchdev.c.
The offending bridge data structures that need this are
br->multicast_lock and br->mdb_list, these are also compiled out of
struct net_bridge when CONFIG_BRIDGE_IGMP_SNOOPING is turned off.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_mdb.c       | 244 ------------------------------------
 net/bridge/br_private.h   |  17 +--
 net/bridge/br_switchdev.c | 253 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 252 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 9513f0791c3d..4556d913955b 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -552,250 +552,6 @@ static size_t rtnl_mdb_nlmsg_size(struct net_bridge_port_group *pg)
 	return nlmsg_size;
 }
 
-struct br_mdb_complete_info {
-	struct net_bridge_port *port;
-	struct br_ip ip;
-};
-
-static void br_mdb_complete(struct net_device *dev, int err, void *priv)
-{
-	struct br_mdb_complete_info *data = priv;
-	struct net_bridge_port_group __rcu **pp;
-	struct net_bridge_port_group *p;
-	struct net_bridge_mdb_entry *mp;
-	struct net_bridge_port *port = data->port;
-	struct net_bridge *br = port->br;
-
-	if (err)
-		goto err;
-
-	spin_lock_bh(&br->multicast_lock);
-	mp = br_mdb_ip_get(br, &data->ip);
-	if (!mp)
-		goto out;
-	for (pp = &mp->ports; (p = mlock_dereference(*pp, br)) != NULL;
-	     pp = &p->next) {
-		if (p->key.port != port)
-			continue;
-		p->flags |= MDB_PG_FLAGS_OFFLOAD;
-	}
-out:
-	spin_unlock_bh(&br->multicast_lock);
-err:
-	kfree(priv);
-}
-
-static void br_switchdev_mdb_populate(struct switchdev_obj_port_mdb *mdb,
-				      const struct net_bridge_mdb_entry *mp)
-{
-	if (mp->addr.proto == htons(ETH_P_IP))
-		ip_eth_mc_map(mp->addr.dst.ip4, mdb->addr);
-#if IS_ENABLED(CONFIG_IPV6)
-	else if (mp->addr.proto == htons(ETH_P_IPV6))
-		ipv6_eth_mc_map(&mp->addr.dst.ip6, mdb->addr);
-#endif
-	else
-		ether_addr_copy(mdb->addr, mp->addr.dst.mac_addr);
-
-	mdb->vid = mp->addr.vid;
-}
-
-static int br_mdb_replay_one(struct notifier_block *nb, struct net_device *dev,
-			     const struct switchdev_obj_port_mdb *mdb,
-			     unsigned long action, const void *ctx,
-			     struct netlink_ext_ack *extack)
-{
-	struct switchdev_notifier_port_obj_info obj_info = {
-		.info = {
-			.dev = dev,
-			.extack = extack,
-			.ctx = ctx,
-		},
-		.obj = &mdb->obj,
-	};
-	int err;
-
-	err = nb->notifier_call(nb, action, &obj_info);
-	return notifier_to_errno(err);
-}
-
-static int br_mdb_queue_one(struct list_head *mdb_list,
-			    enum switchdev_obj_id id,
-			    const struct net_bridge_mdb_entry *mp,
-			    struct net_device *orig_dev)
-{
-	struct switchdev_obj_port_mdb *mdb;
-
-	mdb = kzalloc(sizeof(*mdb), GFP_ATOMIC);
-	if (!mdb)
-		return -ENOMEM;
-
-	mdb->obj.id = id;
-	mdb->obj.orig_dev = orig_dev;
-	br_switchdev_mdb_populate(mdb, mp);
-	list_add_tail(&mdb->obj.list, mdb_list);
-
-	return 0;
-}
-
-int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
-		  const void *ctx, bool adding, struct notifier_block *nb,
-		  struct netlink_ext_ack *extack)
-{
-	const struct net_bridge_mdb_entry *mp;
-	struct switchdev_obj *obj, *tmp;
-	struct net_bridge *br;
-	unsigned long action;
-	LIST_HEAD(mdb_list);
-	int err = 0;
-
-	ASSERT_RTNL();
-
-	if (!nb)
-		return 0;
-
-	if (!netif_is_bridge_master(br_dev) || !netif_is_bridge_port(dev))
-		return -EINVAL;
-
-	br = netdev_priv(br_dev);
-
-	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED))
-		return 0;
-
-	/* We cannot walk over br->mdb_list protected just by the rtnl_mutex,
-	 * because the write-side protection is br->multicast_lock. But we
-	 * need to emulate the [ blocking ] calling context of a regular
-	 * switchdev event, so since both br->multicast_lock and RCU read side
-	 * critical sections are atomic, we have no choice but to pick the RCU
-	 * read side lock, queue up all our events, leave the critical section
-	 * and notify switchdev from blocking context.
-	 */
-	rcu_read_lock();
-
-	hlist_for_each_entry_rcu(mp, &br->mdb_list, mdb_node) {
-		struct net_bridge_port_group __rcu * const *pp;
-		const struct net_bridge_port_group *p;
-
-		if (mp->host_joined) {
-			err = br_mdb_queue_one(&mdb_list,
-					       SWITCHDEV_OBJ_ID_HOST_MDB,
-					       mp, br_dev);
-			if (err) {
-				rcu_read_unlock();
-				goto out_free_mdb;
-			}
-		}
-
-		for (pp = &mp->ports; (p = rcu_dereference(*pp)) != NULL;
-		     pp = &p->next) {
-			if (p->key.port->dev != dev)
-				continue;
-
-			err = br_mdb_queue_one(&mdb_list,
-					       SWITCHDEV_OBJ_ID_PORT_MDB,
-					       mp, dev);
-			if (err) {
-				rcu_read_unlock();
-				goto out_free_mdb;
-			}
-		}
-	}
-
-	rcu_read_unlock();
-
-	if (adding)
-		action = SWITCHDEV_PORT_OBJ_ADD;
-	else
-		action = SWITCHDEV_PORT_OBJ_DEL;
-
-	list_for_each_entry(obj, &mdb_list, list) {
-		err = br_mdb_replay_one(nb, dev, SWITCHDEV_OBJ_PORT_MDB(obj),
-					action, ctx, extack);
-		if (err)
-			goto out_free_mdb;
-	}
-
-out_free_mdb:
-	list_for_each_entry_safe(obj, tmp, &mdb_list, list) {
-		list_del(&obj->list);
-		kfree(SWITCHDEV_OBJ_PORT_MDB(obj));
-	}
-
-	return err;
-}
-
-static void br_mdb_switchdev_host_port(struct net_device *dev,
-				       struct net_device *lower_dev,
-				       struct net_bridge_mdb_entry *mp,
-				       int type)
-{
-	struct switchdev_obj_port_mdb mdb = {
-		.obj = {
-			.id = SWITCHDEV_OBJ_ID_HOST_MDB,
-			.flags = SWITCHDEV_F_DEFER,
-			.orig_dev = dev,
-		},
-	};
-
-	br_switchdev_mdb_populate(&mdb, mp);
-
-	switch (type) {
-	case RTM_NEWMDB:
-		switchdev_port_obj_add(lower_dev, &mdb.obj, NULL);
-		break;
-	case RTM_DELMDB:
-		switchdev_port_obj_del(lower_dev, &mdb.obj);
-		break;
-	}
-}
-
-static void br_mdb_switchdev_host(struct net_device *dev,
-				  struct net_bridge_mdb_entry *mp, int type)
-{
-	struct net_device *lower_dev;
-	struct list_head *iter;
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter)
-		br_mdb_switchdev_host_port(dev, lower_dev, mp, type);
-}
-
-static void br_switchdev_mdb_notify(struct net_device *dev,
-				    struct net_bridge_mdb_entry *mp,
-				    struct net_bridge_port_group *pg,
-				    int type)
-{
-	struct br_mdb_complete_info *complete_info;
-	struct switchdev_obj_port_mdb mdb = {
-		.obj = {
-			.id = SWITCHDEV_OBJ_ID_PORT_MDB,
-			.flags = SWITCHDEV_F_DEFER,
-		},
-	};
-
-	if (!pg)
-		return br_mdb_switchdev_host(dev, mp, type);
-
-	br_switchdev_mdb_populate(&mdb, mp);
-
-	mdb.obj.orig_dev = pg->key.port->dev;
-	switch (type) {
-	case RTM_NEWMDB:
-		complete_info = kmalloc(sizeof(*complete_info), GFP_ATOMIC);
-		if (!complete_info)
-			break;
-		complete_info->port = pg->key.port;
-		complete_info->ip = mp->addr;
-		mdb.obj.complete_priv = complete_info;
-		mdb.obj.complete = br_mdb_complete;
-		if (switchdev_port_obj_add(pg->key.port->dev, &mdb.obj, NULL))
-			kfree(complete_info);
-		break;
-	case RTM_DELMDB:
-		switchdev_port_obj_del(pg->key.port->dev, &mdb.obj);
-		break;
-	}
-}
-
 void br_mdb_notify(struct net_device *dev,
 		   struct net_bridge_mdb_entry *mp,
 		   struct net_bridge_port_group *pg,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b16c83e10356..5552c00ed9c4 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -956,9 +956,11 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 				      struct netlink_ext_ack *extack);
 bool br_multicast_toggle_global_vlan(struct net_bridge_vlan *vlan, bool on);
 
-int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
-		  const void *ctx, bool adding, struct notifier_block *nb,
-		  struct netlink_ext_ack *extack);
+void br_switchdev_mdb_notify(struct net_device *dev,
+			     struct net_bridge_mdb_entry *mp,
+			     struct net_bridge_port_group *pg,
+			     int type);
+
 int br_rports_fill_info(struct sk_buff *skb,
 			const struct net_bridge_mcast *brmctx);
 int br_multicast_dump_querier_state(struct sk_buff *skb,
@@ -1394,12 +1396,11 @@ static inline bool br_multicast_toggle_global_vlan(struct net_bridge_vlan *vlan,
 	return false;
 }
 
-static inline int br_mdb_replay(struct net_device *br_dev,
-				struct net_device *dev, const void *ctx,
-				bool adding, struct notifier_block *nb,
-				struct netlink_ext_ack *extack)
+static inline void br_switchdev_mdb_notify(struct net_device *dev,
+					   struct net_bridge_mdb_entry *mp,
+					   struct net_bridge_port_group *pg,
+					   int type)
 {
-	return -EOPNOTSUPP;
 }
 
 static inline bool
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index d773d819a867..b7645165143c 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -4,6 +4,7 @@
 #include <linux/netdevice.h>
 #include <linux/rtnetlink.h>
 #include <linux/skbuff.h>
+#include <net/ip.h>
 #include <net/switchdev.h>
 
 #include "br_private.h"
@@ -412,6 +413,258 @@ static int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
 	return err;
 }
 
+#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
+struct br_mdb_complete_info {
+	struct net_bridge_port *port;
+	struct br_ip ip;
+};
+
+static void br_mdb_complete(struct net_device *dev, int err, void *priv)
+{
+	struct br_mdb_complete_info *data = priv;
+	struct net_bridge_port_group __rcu **pp;
+	struct net_bridge_port_group *p;
+	struct net_bridge_mdb_entry *mp;
+	struct net_bridge_port *port = data->port;
+	struct net_bridge *br = port->br;
+
+	if (err)
+		goto err;
+
+	spin_lock_bh(&br->multicast_lock);
+	mp = br_mdb_ip_get(br, &data->ip);
+	if (!mp)
+		goto out;
+	for (pp = &mp->ports; (p = mlock_dereference(*pp, br)) != NULL;
+	     pp = &p->next) {
+		if (p->key.port != port)
+			continue;
+		p->flags |= MDB_PG_FLAGS_OFFLOAD;
+	}
+out:
+	spin_unlock_bh(&br->multicast_lock);
+err:
+	kfree(priv);
+}
+
+static void br_switchdev_mdb_populate(struct switchdev_obj_port_mdb *mdb,
+				      const struct net_bridge_mdb_entry *mp)
+{
+	if (mp->addr.proto == htons(ETH_P_IP))
+		ip_eth_mc_map(mp->addr.dst.ip4, mdb->addr);
+#if IS_ENABLED(CONFIG_IPV6)
+	else if (mp->addr.proto == htons(ETH_P_IPV6))
+		ipv6_eth_mc_map(&mp->addr.dst.ip6, mdb->addr);
+#endif
+	else
+		ether_addr_copy(mdb->addr, mp->addr.dst.mac_addr);
+
+	mdb->vid = mp->addr.vid;
+}
+
+static void br_mdb_switchdev_host_port(struct net_device *dev,
+				       struct net_device *lower_dev,
+				       struct net_bridge_mdb_entry *mp,
+				       int type)
+{
+	struct switchdev_obj_port_mdb mdb = {
+		.obj = {
+			.id = SWITCHDEV_OBJ_ID_HOST_MDB,
+			.flags = SWITCHDEV_F_DEFER,
+			.orig_dev = dev,
+		},
+	};
+
+	br_switchdev_mdb_populate(&mdb, mp);
+
+	switch (type) {
+	case RTM_NEWMDB:
+		switchdev_port_obj_add(lower_dev, &mdb.obj, NULL);
+		break;
+	case RTM_DELMDB:
+		switchdev_port_obj_del(lower_dev, &mdb.obj);
+		break;
+	}
+}
+
+static void br_mdb_switchdev_host(struct net_device *dev,
+				  struct net_bridge_mdb_entry *mp, int type)
+{
+	struct net_device *lower_dev;
+	struct list_head *iter;
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter)
+		br_mdb_switchdev_host_port(dev, lower_dev, mp, type);
+}
+
+static int br_mdb_replay_one(struct notifier_block *nb, struct net_device *dev,
+			     const struct switchdev_obj_port_mdb *mdb,
+			     unsigned long action, const void *ctx,
+			     struct netlink_ext_ack *extack)
+{
+	struct switchdev_notifier_port_obj_info obj_info = {
+		.info = {
+			.dev = dev,
+			.extack = extack,
+			.ctx = ctx,
+		},
+		.obj = &mdb->obj,
+	};
+	int err;
+
+	err = nb->notifier_call(nb, action, &obj_info);
+	return notifier_to_errno(err);
+}
+
+static int br_mdb_queue_one(struct list_head *mdb_list,
+			    enum switchdev_obj_id id,
+			    const struct net_bridge_mdb_entry *mp,
+			    struct net_device *orig_dev)
+{
+	struct switchdev_obj_port_mdb *mdb;
+
+	mdb = kzalloc(sizeof(*mdb), GFP_ATOMIC);
+	if (!mdb)
+		return -ENOMEM;
+
+	mdb->obj.id = id;
+	mdb->obj.orig_dev = orig_dev;
+	br_switchdev_mdb_populate(mdb, mp);
+	list_add_tail(&mdb->obj.list, mdb_list);
+
+	return 0;
+}
+
+void br_switchdev_mdb_notify(struct net_device *dev,
+			     struct net_bridge_mdb_entry *mp,
+			     struct net_bridge_port_group *pg,
+			     int type)
+{
+	struct br_mdb_complete_info *complete_info;
+	struct switchdev_obj_port_mdb mdb = {
+		.obj = {
+			.id = SWITCHDEV_OBJ_ID_PORT_MDB,
+			.flags = SWITCHDEV_F_DEFER,
+		},
+	};
+
+	if (!pg)
+		return br_mdb_switchdev_host(dev, mp, type);
+
+	br_switchdev_mdb_populate(&mdb, mp);
+
+	mdb.obj.orig_dev = pg->key.port->dev;
+	switch (type) {
+	case RTM_NEWMDB:
+		complete_info = kmalloc(sizeof(*complete_info), GFP_ATOMIC);
+		if (!complete_info)
+			break;
+		complete_info->port = pg->key.port;
+		complete_info->ip = mp->addr;
+		mdb.obj.complete_priv = complete_info;
+		mdb.obj.complete = br_mdb_complete;
+		if (switchdev_port_obj_add(pg->key.port->dev, &mdb.obj, NULL))
+			kfree(complete_info);
+		break;
+	case RTM_DELMDB:
+		switchdev_port_obj_del(pg->key.port->dev, &mdb.obj);
+		break;
+	}
+}
+#endif
+
+static int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
+			 const void *ctx, bool adding,
+			 struct notifier_block *nb,
+			 struct netlink_ext_ack *extack)
+{
+#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
+	const struct net_bridge_mdb_entry *mp;
+	struct switchdev_obj *obj, *tmp;
+	struct net_bridge *br;
+	unsigned long action;
+	LIST_HEAD(mdb_list);
+	int err = 0;
+
+	ASSERT_RTNL();
+
+	if (!nb)
+		return 0;
+
+	if (!netif_is_bridge_master(br_dev) || !netif_is_bridge_port(dev))
+		return -EINVAL;
+
+	br = netdev_priv(br_dev);
+
+	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED))
+		return 0;
+
+	/* We cannot walk over br->mdb_list protected just by the rtnl_mutex,
+	 * because the write-side protection is br->multicast_lock. But we
+	 * need to emulate the [ blocking ] calling context of a regular
+	 * switchdev event, so since both br->multicast_lock and RCU read side
+	 * critical sections are atomic, we have no choice but to pick the RCU
+	 * read side lock, queue up all our events, leave the critical section
+	 * and notify switchdev from blocking context.
+	 */
+	rcu_read_lock();
+
+	hlist_for_each_entry_rcu(mp, &br->mdb_list, mdb_node) {
+		struct net_bridge_port_group __rcu * const *pp;
+		const struct net_bridge_port_group *p;
+
+		if (mp->host_joined) {
+			err = br_mdb_queue_one(&mdb_list,
+					       SWITCHDEV_OBJ_ID_HOST_MDB,
+					       mp, br_dev);
+			if (err) {
+				rcu_read_unlock();
+				goto out_free_mdb;
+			}
+		}
+
+		for (pp = &mp->ports; (p = rcu_dereference(*pp)) != NULL;
+		     pp = &p->next) {
+			if (p->key.port->dev != dev)
+				continue;
+
+			err = br_mdb_queue_one(&mdb_list,
+					       SWITCHDEV_OBJ_ID_PORT_MDB,
+					       mp, dev);
+			if (err) {
+				rcu_read_unlock();
+				goto out_free_mdb;
+			}
+		}
+	}
+
+	rcu_read_unlock();
+
+	if (adding)
+		action = SWITCHDEV_PORT_OBJ_ADD;
+	else
+		action = SWITCHDEV_PORT_OBJ_DEL;
+
+	list_for_each_entry(obj, &mdb_list, list) {
+		err = br_mdb_replay_one(nb, dev, SWITCHDEV_OBJ_PORT_MDB(obj),
+					action, ctx, extack);
+		if (err)
+			goto out_free_mdb;
+	}
+
+out_free_mdb:
+	list_for_each_entry_safe(obj, tmp, &mdb_list, list) {
+		list_del(&obj->list);
+		kfree(SWITCHDEV_OBJ_PORT_MDB(obj));
+	}
+
+	if (err)
+		return err;
+#endif
+
+	return 0;
+}
+
 static int nbp_switchdev_sync_objs(struct net_bridge_port *p, const void *ctx,
 				   struct notifier_block *atomic_nb,
 				   struct notifier_block *blocking_nb,
-- 
2.25.1

Re: [PATCH net-next 4/5] net: bridge: mdb: move all switchdev logic to br_switchdev.c

[Nikolay Aleksandrov]

On 27/10/2021 19:21, Vladimir Oltean wrote:
> The following functions:
> 
> br_mdb_complete
> br_switchdev_mdb_populate
> br_mdb_replay_one
> br_mdb_queue_one
> br_mdb_replay
> br_mdb_switchdev_host_port
> br_mdb_switchdev_host
> br_switchdev_mdb_notify
> 
> are only accessible from code paths where CONFIG_NET_SWITCHDEV is
> enabled. So move them to br_switchdev.c, in order for that code to be
> compiled out if that config option is disabled.
> 
> Note that br_switchdev.c gets build regardless of whether
> CONFIG_BRIDGE_IGMP_SNOOPING is enabled or not, whereas br_mdb.c only got
> built when CONFIG_BRIDGE_IGMP_SNOOPING was enabled. So to preserve
> correct compilation with CONFIG_BRIDGE_IGMP_SNOOPING being disabled, we
> must now place an #ifdef around these functions in br_switchdev.c.
> The offending bridge data structures that need this are
> br->multicast_lock and br->mdb_list, these are also compiled out of
> struct net_bridge when CONFIG_BRIDGE_IGMP_SNOOPING is turned off.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_mdb.c       | 244 ------------------------------------
>  net/bridge/br_private.h   |  17 +--
>  net/bridge/br_switchdev.c | 253 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 262 insertions(+), 252 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>

[PATCH net-next 5/5] net: bridge: switchdev: consistent function naming

[Vladimir Oltean]

Rename all recently imported functions in br_switchdev.c to start with a
br_switchdev_* prefix.

br_fdb_replay_one() -> br_switchdev_fdb_replay_one()
br_fdb_replay() -> br_switchdev_fdb_replay()
br_vlan_replay_one() -> br_switchdev_vlan_replay_one()
br_vlan_replay() -> br_switchdev_vlan_replay()
struct br_mdb_complete_info -> struct br_switchdev_mdb_complete_info
br_mdb_complete() -> br_switchdev_mdb_complete()
br_mdb_switchdev_host_port() -> br_switchdev_host_mdb_one()
br_mdb_switchdev_host() -> br_switchdev_host_mdb()
br_mdb_replay_one() -> br_switchdev_mdb_replay_one()
br_mdb_replay() -> br_switchdev_mdb_replay()
br_mdb_queue_one() -> br_switchdev_mdb_queue_one()

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_switchdev.c | 117 ++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 54 deletions(-)

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index b7645165143c..f8fbaaa7c501 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -281,9 +281,10 @@ static void nbp_switchdev_del(struct net_bridge_port *p)
 	}
 }
 
-static int br_fdb_replay_one(struct net_bridge *br, struct notifier_block *nb,
-			     const struct net_bridge_fdb_entry *fdb,
-			     unsigned long action, const void *ctx)
+static int
+br_switchdev_fdb_replay_one(struct net_bridge *br, struct notifier_block *nb,
+			    const struct net_bridge_fdb_entry *fdb,
+			    unsigned long action, const void *ctx)
 {
 	struct switchdev_notifier_fdb_info item;
 	int err;
@@ -294,8 +295,9 @@ static int br_fdb_replay_one(struct net_bridge *br, struct notifier_block *nb,
 	return notifier_to_errno(err);
 }
 
-static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
-			 bool adding, struct notifier_block *nb)
+static int
+br_switchdev_fdb_replay(const struct net_device *br_dev, const void *ctx,
+			bool adding, struct notifier_block *nb)
 {
 	struct net_bridge_fdb_entry *fdb;
 	struct net_bridge *br;
@@ -318,7 +320,7 @@ static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
 	rcu_read_lock();
 
 	hlist_for_each_entry_rcu(fdb, &br->fdb_list, fdb_node) {
-		err = br_fdb_replay_one(br, nb, fdb, action, ctx);
+		err = br_switchdev_fdb_replay_one(br, nb, fdb, action, ctx);
 		if (err)
 			break;
 	}
@@ -328,11 +330,12 @@ static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
 	return err;
 }
 
-static int br_vlan_replay_one(struct notifier_block *nb,
-			      struct net_device *dev,
-			      struct switchdev_obj_port_vlan *vlan,
-			      const void *ctx, unsigned long action,
-			      struct netlink_ext_ack *extack)
+static int
+br_switchdev_vlan_replay_one(struct notifier_block *nb,
+			     struct net_device *dev,
+			     struct switchdev_obj_port_vlan *vlan,
+			     const void *ctx, unsigned long action,
+			     struct netlink_ext_ack *extack)
 {
 	struct switchdev_notifier_port_obj_info obj_info = {
 		.info = {
@@ -348,10 +351,11 @@ static int br_vlan_replay_one(struct notifier_block *nb,
 	return notifier_to_errno(err);
 }
 
-static int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
-			  const void *ctx, bool adding,
-			  struct notifier_block *nb,
-			  struct netlink_ext_ack *extack)
+static int br_switchdev_vlan_replay(struct net_device *br_dev,
+				    struct net_device *dev,
+				    const void *ctx, bool adding,
+				    struct notifier_block *nb,
+				    struct netlink_ext_ack *extack)
 {
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_vlan *v;
@@ -405,7 +409,8 @@ static int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
 		if (!br_vlan_should_use(v))
 			continue;
 
-		err = br_vlan_replay_one(nb, dev, &vlan, ctx, action, extack);
+		err = br_switchdev_vlan_replay_one(nb, dev, &vlan, ctx,
+						   action, extack);
 		if (err)
 			return err;
 	}
@@ -414,14 +419,14 @@ static int br_vlan_replay(struct net_device *br_dev, struct net_device *dev,
 }
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
-struct br_mdb_complete_info {
+struct br_switchdev_mdb_complete_info {
 	struct net_bridge_port *port;
 	struct br_ip ip;
 };
 
-static void br_mdb_complete(struct net_device *dev, int err, void *priv)
+static void br_switchdev_mdb_complete(struct net_device *dev, int err, void *priv)
 {
-	struct br_mdb_complete_info *data = priv;
+	struct br_switchdev_mdb_complete_info *data = priv;
 	struct net_bridge_port_group __rcu **pp;
 	struct net_bridge_port_group *p;
 	struct net_bridge_mdb_entry *mp;
@@ -462,10 +467,10 @@ static void br_switchdev_mdb_populate(struct switchdev_obj_port_mdb *mdb,
 	mdb->vid = mp->addr.vid;
 }
 
-static void br_mdb_switchdev_host_port(struct net_device *dev,
-				       struct net_device *lower_dev,
-				       struct net_bridge_mdb_entry *mp,
-				       int type)
+static void br_switchdev_host_mdb_one(struct net_device *dev,
+				      struct net_device *lower_dev,
+				      struct net_bridge_mdb_entry *mp,
+				      int type)
 {
 	struct switchdev_obj_port_mdb mdb = {
 		.obj = {
@@ -487,20 +492,21 @@ static void br_mdb_switchdev_host_port(struct net_device *dev,
 	}
 }
 
-static void br_mdb_switchdev_host(struct net_device *dev,
+static void br_switchdev_host_mdb(struct net_device *dev,
 				  struct net_bridge_mdb_entry *mp, int type)
 {
 	struct net_device *lower_dev;
 	struct list_head *iter;
 
 	netdev_for_each_lower_dev(dev, lower_dev, iter)
-		br_mdb_switchdev_host_port(dev, lower_dev, mp, type);
+		br_switchdev_host_mdb_one(dev, lower_dev, mp, type);
 }
 
-static int br_mdb_replay_one(struct notifier_block *nb, struct net_device *dev,
-			     const struct switchdev_obj_port_mdb *mdb,
-			     unsigned long action, const void *ctx,
-			     struct netlink_ext_ack *extack)
+static int
+br_switchdev_mdb_replay_one(struct notifier_block *nb, struct net_device *dev,
+			    const struct switchdev_obj_port_mdb *mdb,
+			    unsigned long action, const void *ctx,
+			    struct netlink_ext_ack *extack)
 {
 	struct switchdev_notifier_port_obj_info obj_info = {
 		.info = {
@@ -516,10 +522,10 @@ static int br_mdb_replay_one(struct notifier_block *nb, struct net_device *dev,
 	return notifier_to_errno(err);
 }
 
-static int br_mdb_queue_one(struct list_head *mdb_list,
-			    enum switchdev_obj_id id,
-			    const struct net_bridge_mdb_entry *mp,
-			    struct net_device *orig_dev)
+static int br_switchdev_mdb_queue_one(struct list_head *mdb_list,
+				      enum switchdev_obj_id id,
+				      const struct net_bridge_mdb_entry *mp,
+				      struct net_device *orig_dev)
 {
 	struct switchdev_obj_port_mdb *mdb;
 
@@ -540,7 +546,7 @@ void br_switchdev_mdb_notify(struct net_device *dev,
 			     struct net_bridge_port_group *pg,
 			     int type)
 {
-	struct br_mdb_complete_info *complete_info;
+	struct br_switchdev_mdb_complete_info *complete_info;
 	struct switchdev_obj_port_mdb mdb = {
 		.obj = {
 			.id = SWITCHDEV_OBJ_ID_PORT_MDB,
@@ -549,7 +555,7 @@ void br_switchdev_mdb_notify(struct net_device *dev,
 	};
 
 	if (!pg)
-		return br_mdb_switchdev_host(dev, mp, type);
+		return br_switchdev_host_mdb(dev, mp, type);
 
 	br_switchdev_mdb_populate(&mdb, mp);
 
@@ -562,7 +568,7 @@ void br_switchdev_mdb_notify(struct net_device *dev,
 		complete_info->port = pg->key.port;
 		complete_info->ip = mp->addr;
 		mdb.obj.complete_priv = complete_info;
-		mdb.obj.complete = br_mdb_complete;
+		mdb.obj.complete = br_switchdev_mdb_complete;
 		if (switchdev_port_obj_add(pg->key.port->dev, &mdb.obj, NULL))
 			kfree(complete_info);
 		break;
@@ -573,10 +579,10 @@ void br_switchdev_mdb_notify(struct net_device *dev,
 }
 #endif
 
-static int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
-			 const void *ctx, bool adding,
-			 struct notifier_block *nb,
-			 struct netlink_ext_ack *extack)
+static int
+br_switchdev_mdb_replay(struct net_device *br_dev, struct net_device *dev,
+			const void *ctx, bool adding, struct notifier_block *nb,
+			struct netlink_ext_ack *extack)
 {
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	const struct net_bridge_mdb_entry *mp;
@@ -614,9 +620,9 @@ static int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
 		const struct net_bridge_port_group *p;
 
 		if (mp->host_joined) {
-			err = br_mdb_queue_one(&mdb_list,
-					       SWITCHDEV_OBJ_ID_HOST_MDB,
-					       mp, br_dev);
+			err = br_switchdev_mdb_queue_one(&mdb_list,
+							 SWITCHDEV_OBJ_ID_HOST_MDB,
+							 mp, br_dev);
 			if (err) {
 				rcu_read_unlock();
 				goto out_free_mdb;
@@ -628,9 +634,9 @@ static int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
 			if (p->key.port->dev != dev)
 				continue;
 
-			err = br_mdb_queue_one(&mdb_list,
-					       SWITCHDEV_OBJ_ID_PORT_MDB,
-					       mp, dev);
+			err = br_switchdev_mdb_queue_one(&mdb_list,
+							 SWITCHDEV_OBJ_ID_PORT_MDB,
+							 mp, dev);
 			if (err) {
 				rcu_read_unlock();
 				goto out_free_mdb;
@@ -646,8 +652,9 @@ static int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
 		action = SWITCHDEV_PORT_OBJ_DEL;
 
 	list_for_each_entry(obj, &mdb_list, list) {
-		err = br_mdb_replay_one(nb, dev, SWITCHDEV_OBJ_PORT_MDB(obj),
-					action, ctx, extack);
+		err = br_switchdev_mdb_replay_one(nb, dev,
+						  SWITCHDEV_OBJ_PORT_MDB(obj),
+						  action, ctx, extack);
 		if (err)
 			goto out_free_mdb;
 	}
@@ -674,15 +681,17 @@ static int nbp_switchdev_sync_objs(struct net_bridge_port *p, const void *ctx,
 	struct net_device *dev = p->dev;
 	int err;
 
-	err = br_vlan_replay(br_dev, dev, ctx, true, blocking_nb, extack);
+	err = br_switchdev_vlan_replay(br_dev, dev, ctx, true, blocking_nb,
+				       extack);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
-	err = br_mdb_replay(br_dev, dev, ctx, true, blocking_nb, extack);
+	err = br_switchdev_mdb_replay(br_dev, dev, ctx, true, blocking_nb,
+				      extack);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
-	err = br_fdb_replay(br_dev, ctx, true, atomic_nb);
+	err = br_switchdev_fdb_replay(br_dev, ctx, true, atomic_nb);
 	if (err && err != -EOPNOTSUPP)
 		return err;
 
@@ -697,11 +706,11 @@ static void nbp_switchdev_unsync_objs(struct net_bridge_port *p,
 	struct net_device *br_dev = p->br->dev;
 	struct net_device *dev = p->dev;
 
-	br_vlan_replay(br_dev, dev, ctx, false, blocking_nb, NULL);
+	br_switchdev_vlan_replay(br_dev, dev, ctx, false, blocking_nb, NULL);
 
-	br_mdb_replay(br_dev, dev, ctx, false, blocking_nb, NULL);
+	br_switchdev_mdb_replay(br_dev, dev, ctx, false, blocking_nb, NULL);
 
-	br_fdb_replay(br_dev, ctx, false, atomic_nb);
+	br_switchdev_fdb_replay(br_dev, ctx, false, atomic_nb);
 }
 
 /* Let the bridge know that this port is offloaded, so that it can assign a
-- 
2.25.1

Re: [PATCH net-next 5/5] net: bridge: switchdev: consistent function naming

[Nikolay Aleksandrov]

On 27/10/2021 19:21, Vladimir Oltean wrote:
> Rename all recently imported functions in br_switchdev.c to start with a
> br_switchdev_* prefix.
> 
> br_fdb_replay_one() -> br_switchdev_fdb_replay_one()
> br_fdb_replay() -> br_switchdev_fdb_replay()
> br_vlan_replay_one() -> br_switchdev_vlan_replay_one()
> br_vlan_replay() -> br_switchdev_vlan_replay()
> struct br_mdb_complete_info -> struct br_switchdev_mdb_complete_info
> br_mdb_complete() -> br_switchdev_mdb_complete()
> br_mdb_switchdev_host_port() -> br_switchdev_host_mdb_one()
> br_mdb_switchdev_host() -> br_switchdev_host_mdb()
> br_mdb_replay_one() -> br_switchdev_mdb_replay_one()
> br_mdb_replay() -> br_switchdev_mdb_replay()
> br_mdb_queue_one() -> br_switchdev_mdb_queue_one()
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  net/bridge/br_switchdev.c | 117 ++++++++++++++++++++------------------
>  1 file changed, 63 insertions(+), 54 deletions(-)
> 

Thanks,
Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 27 Oct 2021 19:21:14 +0300 you wrote:
> This is one more refactoring patch set for the Linux bridge, where more
> logic that is specific to switchdev is moved into br_switchdev.c, which
> is compiled out when CONFIG_NET_SWITCHDEV is disabled.
> 
> Vladimir Oltean (5):
>   net: bridge: provide shim definition for br_vlan_flags
>   net: bridge: move br_vlan_replay to br_switchdev.c
>   net: bridge: split out the switchdev portion of br_mdb_notify
>   net: bridge: mdb: move all switchdev logic to br_switchdev.c
>   net: bridge: switchdev: consistent function naming
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] net: bridge: provide shim definition for br_vlan_flags
    https://git.kernel.org/netdev/net-next/c/c5f6e5ebc2af
  - [net-next,2/5] net: bridge: move br_vlan_replay to br_switchdev.c
    https://git.kernel.org/netdev/net-next/c/4a6849e46173
  - [net-next,3/5] net: bridge: split out the switchdev portion of br_mdb_notify
    https://git.kernel.org/netdev/net-next/c/9ae9ff994b0e
  - [net-next,4/5] net: bridge: mdb: move all switchdev logic to br_switchdev.c
    https://git.kernel.org/netdev/net-next/c/9776457c784f
  - [net-next,5/5] net: bridge: switchdev: consistent function naming
    https://git.kernel.org/netdev/net-next/c/326b212e9cd6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Jiri Pirko]

Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
>This is one more refactoring patch set for the Linux bridge, where more
>logic that is specific to switchdev is moved into br_switchdev.c, which
>is compiled out when CONFIG_NET_SWITCHDEV is disabled.

Looks good.

While you are at it, don't you plan to also move switchdev.c into
br_switchdev.c and eventually rename to br_offload.c ?

Switchdev is about bridge offloading only anyway.


>
>Vladimir Oltean (5):
>  net: bridge: provide shim definition for br_vlan_flags
>  net: bridge: move br_vlan_replay to br_switchdev.c
>  net: bridge: split out the switchdev portion of br_mdb_notify
>  net: bridge: mdb: move all switchdev logic to br_switchdev.c
>  net: bridge: switchdev: consistent function naming
>
> net/bridge/br_mdb.c       | 238 +-----------------------
> net/bridge/br_private.h   |  28 ++-
> net/bridge/br_switchdev.c | 371 ++++++++++++++++++++++++++++++++++++--
> net/bridge/br_vlan.c      |  84 ---------
> 4 files changed, 372 insertions(+), 349 deletions(-)
>
>-- 
>2.25.1
>

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Vladimir Oltean]

On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
> >This is one more refactoring patch set for the Linux bridge, where more
> >logic that is specific to switchdev is moved into br_switchdev.c, which
> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
> 
> Looks good.
> 
> While you are at it, don't you plan to also move switchdev.c into
> br_switchdev.c and eventually rename to br_offload.c ?
> 
> Switchdev is about bridge offloading only anyway.

You mean I should effectively make switchdev part of the bridge?
See commit 957e2235e526 ("net: make switchdev_bridge_port_{,unoffload}
loosely coupled with the bridge").

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Jiri Pirko]

Tue, Nov 02, 2021 at 12:11:59PM CET, vladimir.oltean@nxp.com wrote:
>On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
>> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
>> >This is one more refactoring patch set for the Linux bridge, where more
>> >logic that is specific to switchdev is moved into br_switchdev.c, which
>> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
>> 
>> Looks good.
>> 
>> While you are at it, don't you plan to also move switchdev.c into
>> br_switchdev.c and eventually rename to br_offload.c ?
>> 
>> Switchdev is about bridge offloading only anyway.
>
>You mean I should effectively make switchdev part of the bridge?

Yes.

>See commit 957e2235e526 ("net: make switchdev_bridge_port_{,unoffload}
>loosely coupled with the bridge").

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Vladimir Oltean]

On Tue, Nov 02, 2021 at 12:49:53PM +0100, Jiri Pirko wrote:
> Tue, Nov 02, 2021 at 12:11:59PM CET, vladimir.oltean@nxp.com wrote:
> >On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
> >> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
> >> >This is one more refactoring patch set for the Linux bridge, where more
> >> >logic that is specific to switchdev is moved into br_switchdev.c, which
> >> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
> >> 
> >> Looks good.
> >> 
> >> While you are at it, don't you plan to also move switchdev.c into
> >> br_switchdev.c and eventually rename to br_offload.c ?
> >> 
> >> Switchdev is about bridge offloading only anyway.
> >
> >You mean I should effectively make switchdev part of the bridge?
> 
> Yes.

Ok, have you actually seen the commit message linked below? Basically it
says that there are drivers that depend on switchdev.c being this
neutral third party, forwarding events on notifier chains back and forth
between the bridge and the drivers. If we make switchdev.c part of the
bridge, then drivers can no longer be compiled without bridge support.
Currently br_switchdev.c is compiled as follows:

bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o

whereas switchdev.c is compiled as follows:

obj-y += switchdev.o

So to rephrase the question: unless you're suggesting that I should
build br_switchdev.o as part of obj-$(CONFIG_NET_SWITCHDEV) instead of
bridge-$(CONFIG_NET_SWITCHDEV), then what do we do with the drivers that
assume that the switchdev functions they call into are present when the
bridge is not compiled, or is compiled as module?

> >See commit 957e2235e526 ("net: make switchdev_bridge_port_{,unoffload}
> >loosely coupled with the bridge").

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Jiri Pirko]

Tue, Nov 02, 2021 at 01:02:06PM CET, vladimir.oltean@nxp.com wrote:
>On Tue, Nov 02, 2021 at 12:49:53PM +0100, Jiri Pirko wrote:
>> Tue, Nov 02, 2021 at 12:11:59PM CET, vladimir.oltean@nxp.com wrote:
>> >On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
>> >> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
>> >> >This is one more refactoring patch set for the Linux bridge, where more
>> >> >logic that is specific to switchdev is moved into br_switchdev.c, which
>> >> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
>> >> 
>> >> Looks good.
>> >> 
>> >> While you are at it, don't you plan to also move switchdev.c into
>> >> br_switchdev.c and eventually rename to br_offload.c ?
>> >> 
>> >> Switchdev is about bridge offloading only anyway.
>> >
>> >You mean I should effectively make switchdev part of the bridge?
>> 
>> Yes.
>
>Ok, have you actually seen the commit message linked below? Basically it
>says that there are drivers that depend on switchdev.c being this
>neutral third party, forwarding events on notifier chains back and forth
>between the bridge and the drivers. If we make switchdev.c part of the
>bridge, then drivers can no longer be compiled without bridge support.
>Currently br_switchdev.c is compiled as follows:
>
>bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o
>
>whereas switchdev.c is compiled as follows:
>
>obj-y += switchdev.o
>
>So to rephrase the question: unless you're suggesting that I should
>build br_switchdev.o as part of obj-$(CONFIG_NET_SWITCHDEV) instead of
>bridge-$(CONFIG_NET_SWITCHDEV), then what do we do with the drivers that
>assume that the switchdev functions they call into are present when the
>bridge is not compiled, or is compiled as module?

It can work similarly as VLAN. It also has "vlan core" which is always
in for drivers, however VLAN driver itself does not have to be.


>
>> >See commit 957e2235e526 ("net: make switchdev_bridge_port_{,unoffload}
>> >loosely coupled with the bridge").

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Andrew Lunn]

On Tue, Nov 02, 2021 at 12:02:06PM +0000, Vladimir Oltean wrote:
> On Tue, Nov 02, 2021 at 12:49:53PM +0100, Jiri Pirko wrote:
> > Tue, Nov 02, 2021 at 12:11:59PM CET, vladimir.oltean@nxp.com wrote:
> > >On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
> > >> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
> > >> >This is one more refactoring patch set for the Linux bridge, where more
> > >> >logic that is specific to switchdev is moved into br_switchdev.c, which
> > >> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
> > >> 
> > >> Looks good.
> > >> 
> > >> While you are at it, don't you plan to also move switchdev.c into
> > >> br_switchdev.c and eventually rename to br_offload.c ?
> > >> 
> > >> Switchdev is about bridge offloading only anyway.
> > >
> > >You mean I should effectively make switchdev part of the bridge?
> > 
> > Yes.
> 
> Ok, have you actually seen the commit message linked below? Basically it
> says that there are drivers that depend on switchdev.c being this
> neutral third party, forwarding events on notifier chains back and forth
> between the bridge and the drivers. If we make switchdev.c part of the
> bridge, then drivers can no longer be compiled without bridge support.

This is something i test every so often, building without the
bridge. The simplest DSA drivers just provide a 'port multiplexor', no
offload at all. You can put IP addresses on the interfaces and
software route between them etc.

So i would prefer this use case does not break.

   Andrew

Re: [PATCH net-next 0/5] Code movement to br_switchdev.c

[Vladimir Oltean]

On Tue, Nov 02, 2021 at 06:03:33PM +0100, Andrew Lunn wrote:
> On Tue, Nov 02, 2021 at 12:02:06PM +0000, Vladimir Oltean wrote:
> > On Tue, Nov 02, 2021 at 12:49:53PM +0100, Jiri Pirko wrote:
> > > Tue, Nov 02, 2021 at 12:11:59PM CET, vladimir.oltean@nxp.com wrote:
> > > >On Mon, Nov 01, 2021 at 04:05:45PM +0100, Jiri Pirko wrote:
> > > >> Wed, Oct 27, 2021 at 06:21:14PM CEST, vladimir.oltean@nxp.com wrote:
> > > >> >This is one more refactoring patch set for the Linux bridge, where more
> > > >> >logic that is specific to switchdev is moved into br_switchdev.c, which
> > > >> >is compiled out when CONFIG_NET_SWITCHDEV is disabled.
> > > >> 
> > > >> Looks good.
> > > >> 
> > > >> While you are at it, don't you plan to also move switchdev.c into
> > > >> br_switchdev.c and eventually rename to br_offload.c ?
> > > >> 
> > > >> Switchdev is about bridge offloading only anyway.
> > > >
> > > >You mean I should effectively make switchdev part of the bridge?
> > > 
> > > Yes.
> > 
> > Ok, have you actually seen the commit message linked below? Basically it
> > says that there are drivers that depend on switchdev.c being this
> > neutral third party, forwarding events on notifier chains back and forth
> > between the bridge and the drivers. If we make switchdev.c part of the
> > bridge, then drivers can no longer be compiled without bridge support.
> 
> This is something i test every so often, building without the
> bridge. The simplest DSA drivers just provide a 'port multiplexor', no
> offload at all. You can put IP addresses on the interfaces and
> software route between them etc.
> 
> So i would prefer this use case does not break.

I should have formulated it more carefully. That use case is not broken.
What would break would be the ability to compile drivers (in this case DSA)
as built-in if the bridge is a module.