Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1472102 > unrolled thread

[PATCH net-next 0/3] net: dsa: add MDB support

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next 0/3] net: dsa: add MDB support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-29 22:40 +0200
    [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-29 22:40 +0200
      Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops  generic Andrew Lunn <andrew@lunn.ch> - 2016-08-31 15:50 +0200
        Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-31 16:40 +0200
          Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops  generic Andrew Lunn <andrew@lunn.ch> - 2016-08-31 17:00 +0200
            Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-31 17:10 +0200
    [PATCH net-next 1/3] net: dsa: add MDB support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-29 22:40 +0200
      Re: [PATCH net-next 1/3] net: dsa: add MDB support Andrew Lunn <andrew@lunn.ch> - 2016-08-31 15:30 +0200
        Re: [PATCH net-next 1/3] net: dsa: add MDB support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-31 16:30 +0200
          Re: [PATCH net-next 1/3] net: dsa: add MDB support Andrew Lunn <andrew@lunn.ch> - 2016-08-31 16:40 +0200
            Re: [PATCH net-next 1/3] net: dsa: add MDB support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-31 17:00 +0200
              Re: [PATCH net-next 1/3] net: dsa: add MDB support Andrew Lunn <andrew@lunn.ch> - 2016-08-31 17:10 +0200

#1472102 — [PATCH net-next 0/3] net: dsa: add MDB support

This patchset adds the switchdev MDB object support to the DSA layer.

The MDB support for the mv88e6xxx driver is very similar to the FDB
support. The FDB operations care about unicast addresses while the MDB
operations care about multicast addresses.

Both operation set load/purge/dump the Address Translation Table (ATU),
thus common code is used.

Vivien Didelot (3):
  net: dsa: add MDB support
  net: dsa: mv88e6xxx: make switchdev DB ops generic
  net: dsa: mv88e6xxx: add MDB support

 Documentation/networking/dsa/dsa.txt |  23 +++++
 drivers/net/dsa/mv88e6xxx/chip.c     | 161 ++++++++++++++++++++++++++---------
 include/net/dsa.h                    |  16 ++++
 net/dsa/slave.c                      |  55 ++++++++++++
 4 files changed, 213 insertions(+), 42 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1472105 — [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic

The MDB support for the mv88e6xxx driver will be very similar to the FDB
support, since it consists of loading/purging/dumping address to/from
the Address Translation Unit (ATU).

Prepare the support for MDB by making the FDB code accessing the ATU
generic. The FDB operations now provide access to the unicast addresses
while the MDB operations will provide access to the multicast addresses.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 98 ++++++++++++++++++++++------------------
 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 750d01d..93abfff 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2093,9 +2093,9 @@ static int _mv88e6xxx_atu_load(struct mv88e6xxx_chip *chip,
 	return _mv88e6xxx_atu_cmd(chip, entry->fid, GLOBAL_ATU_OP_LOAD_DB);
 }
 
-static int _mv88e6xxx_port_fdb_load(struct mv88e6xxx_chip *chip, int port,
-				    const unsigned char *addr, u16 vid,
-				    u8 state)
+static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
+					const unsigned char *addr, u16 vid,
+					u8 state)
 {
 	struct mv88e6xxx_atu_entry entry = { 0 };
 	struct mv88e6xxx_vtu_stu_entry vlan;
@@ -2134,15 +2134,12 @@ static void mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 				   const struct switchdev_obj_port_fdb *fdb,
 				   struct switchdev_trans *trans)
 {
-	int state = is_multicast_ether_addr(fdb->addr) ?
-		GLOBAL_ATU_DATA_STATE_MC_STATIC :
-		GLOBAL_ATU_DATA_STATE_UC_STATIC;
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 
 	mutex_lock(&chip->reg_lock);
-	if (_mv88e6xxx_port_fdb_load(chip, port, fdb->addr, fdb->vid, state))
-		netdev_err(ds->ports[port].netdev,
-			   "failed to load MAC address\n");
+	if (mv88e6xxx_port_db_load_purge(chip, port, fdb->addr, fdb->vid,
+					 GLOBAL_ATU_DATA_STATE_UC_STATIC))
+		netdev_err(ds->ports[port].netdev, "failed to load unicast MAC address\n");
 	mutex_unlock(&chip->reg_lock);
 }
 
@@ -2150,14 +2147,14 @@ static int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
 				  const struct switchdev_obj_port_fdb *fdb)
 {
 	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
-	int ret;
+	int err;
 
 	mutex_lock(&chip->reg_lock);
-	ret = _mv88e6xxx_port_fdb_load(chip, port, fdb->addr, fdb->vid,
-				       GLOBAL_ATU_DATA_STATE_UNUSED);
+	err = mv88e6xxx_port_db_load_purge(chip, port, fdb->addr, fdb->vid,
+					   GLOBAL_ATU_DATA_STATE_UNUSED);
 	mutex_unlock(&chip->reg_lock);
 
-	return ret;
+	return err;
 }
 
 static int _mv88e6xxx_atu_getnext(struct mv88e6xxx_chip *chip, u16 fid,
@@ -2205,10 +2202,10 @@ static int _mv88e6xxx_atu_getnext(struct mv88e6xxx_chip *chip, u16 fid,
 	return 0;
 }
 
-static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
-					u16 fid, u16 vid, int port,
-					struct switchdev_obj_port_fdb *fdb,
-					int (*cb)(struct switchdev_obj *obj))
+static int mv88e6xxx_port_db_dump_one(struct mv88e6xxx_chip *chip,
+				      u16 fid, u16 vid, int port,
+				      struct switchdev_obj *obj,
+				      int (*cb)(struct switchdev_obj *obj))
 {
 	struct mv88e6xxx_atu_entry addr = {
 		.mac = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
@@ -2222,72 +2219,87 @@ static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
 	do {
 		err = _mv88e6xxx_atu_getnext(chip, fid, &addr);
 		if (err)
-			break;
+			return err;
 
 		if (addr.state == GLOBAL_ATU_DATA_STATE_UNUSED)
 			break;
 
-		if (!addr.trunk && addr.portv_trunkid & BIT(port)) {
-			bool is_static = addr.state ==
-				(is_multicast_ether_addr(addr.mac) ?
-				 GLOBAL_ATU_DATA_STATE_MC_STATIC :
-				 GLOBAL_ATU_DATA_STATE_UC_STATIC);
+		if (addr.trunk || (addr.portv_trunkid & BIT(port)) == 0)
+			continue;
+
+		if (obj->id == SWITCHDEV_OBJ_ID_PORT_FDB) {
+			struct switchdev_obj_port_fdb *fdb;
 
+			if (!is_unicast_ether_addr(addr.mac))
+				continue;
+
+			fdb = SWITCHDEV_OBJ_PORT_FDB(obj);
 			fdb->vid = vid;
 			ether_addr_copy(fdb->addr, addr.mac);
-			fdb->ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
-
-			err = cb(&fdb->obj);
-			if (err)
-				break;
+			if (addr.state == GLOBAL_ATU_DATA_STATE_UC_STATIC)
+				fdb->ndm_state = NUD_NOARP;
+			else
+				fdb->ndm_state = NUD_REACHABLE;
 		}
+
+		err = cb(obj);
+		if (err)
+			return err;
 	} while (!is_broadcast_ether_addr(addr.mac));
 
 	return err;
 }
 
-static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
-				   struct switchdev_obj_port_fdb *fdb,
-				   int (*cb)(struct switchdev_obj *obj))
+static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
+				  struct switchdev_obj *obj,
+				  int (*cb)(struct switchdev_obj *obj))
 {
-	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
 	struct mv88e6xxx_vtu_stu_entry vlan = {
 		.vid = GLOBAL_VTU_VID_MASK, /* all ones */
 	};
 	u16 fid;
 	int err;
 
-	mutex_lock(&chip->reg_lock);
-
 	/* Dump port's default Filtering Information Database (VLAN ID 0) */
 	err = _mv88e6xxx_port_fid_get(chip, port, &fid);
 	if (err)
-		goto unlock;
+		return err;
 
-	err = _mv88e6xxx_port_fdb_dump_one(chip, fid, 0, port, fdb, cb);
+	err = mv88e6xxx_port_db_dump_one(chip, fid, 0, port, obj, cb);
 	if (err)
-		goto unlock;
+		return err;
 
 	/* Dump VLANs' Filtering Information Databases */
 	err = _mv88e6xxx_vtu_vid_write(chip, vlan.vid);
 	if (err)
-		goto unlock;
+		return err;
 
 	do {
 		err = _mv88e6xxx_vtu_getnext(chip, &vlan);
 		if (err)
-			break;
+			return err;
 
 		if (!vlan.valid)
 			break;
 
-		err = _mv88e6xxx_port_fdb_dump_one(chip, vlan.fid, vlan.vid,
-						   port, fdb, cb);
+		err = mv88e6xxx_port_db_dump_one(chip, vlan.fid, vlan.vid, port,
+						 obj, cb);
 		if (err)
-			break;
+			return err;
 	} while (vlan.vid < GLOBAL_VTU_VID_MASK);
 
-unlock:
+	return err;
+}
+
+static int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
+				   struct switchdev_obj_port_fdb *fdb,
+				   int (*cb)(struct switchdev_obj *obj))
+{
+	struct mv88e6xxx_chip *chip = ds_to_priv(ds);
+	int err;
+
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_port_db_dump(chip, port, &fdb->obj, cb);
 	mutex_unlock(&chip->reg_lock);
 
 	return err;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1473489 — Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic

Hi Vivien

> -static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
> -					u16 fid, u16 vid, int port,
> -					struct switchdev_obj_port_fdb *fdb,
> -					int (*cb)(struct switchdev_obj *obj))
> +static int mv88e6xxx_port_db_dump_one(struct mv88e6xxx_chip *chip,
> +				      u16 fid, u16 vid, int port,
> +				      struct switchdev_obj *obj,
> +				      int (*cb)(struct switchdev_obj *obj))
>  {
>  	struct mv88e6xxx_atu_entry addr = {
>  		.mac = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
> @@ -2222,72 +2219,87 @@ static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
>  	do {
>  		err = _mv88e6xxx_atu_getnext(chip, fid, &addr);
>  		if (err)
> -			break;
> +			return err;
>  
>  		if (addr.state == GLOBAL_ATU_DATA_STATE_UNUSED)
>  			break;
>  
> -		if (!addr.trunk && addr.portv_trunkid & BIT(port)) {
> -			bool is_static = addr.state ==
> -				(is_multicast_ether_addr(addr.mac) ?
> -				 GLOBAL_ATU_DATA_STATE_MC_STATIC :
> -				 GLOBAL_ATU_DATA_STATE_UC_STATIC);
> +		if (addr.trunk || (addr.portv_trunkid & BIT(port)) == 0)
> +			continue;
> +
> +		if (obj->id == SWITCHDEV_OBJ_ID_PORT_FDB) {
> +			struct switchdev_obj_port_fdb *fdb;
>  
> +			if (!is_unicast_ether_addr(addr.mac))
> +				continue;
> +
> +			fdb = SWITCHDEV_OBJ_PORT_FDB(obj);
>  			fdb->vid = vid;
>  			ether_addr_copy(fdb->addr, addr.mac);
> -			fdb->ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
> -
> -			err = cb(&fdb->obj);
> -			if (err)
> -				break;
> +			if (addr.state == GLOBAL_ATU_DATA_STATE_UC_STATIC)
> +				fdb->ndm_state = NUD_NOARP;
> +			else
> +				fdb->ndm_state = NUD_REACHABLE;
>  		}
> +
> +		err = cb(obj);
> +		if (err)
> +			return err;
>  	} while (!is_broadcast_ether_addr(addr.mac));

Humm, maybe i'm reading this patch wrong....

This function is called mv88e6xxx_port_db_dump_one(). But i don't see
a way out of the while loop, after dumping one. It seems to dump the
whole table until it reaches the end marker, which is the MAC
broadcast address.

Should we rename this function, drop the _one?

       Andrew

[toc] | [prev] | [next] | [standalone]

#1473545 — Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> Hi Vivien
>
>> -static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
>> -					u16 fid, u16 vid, int port,
>> -					struct switchdev_obj_port_fdb *fdb,
>> -					int (*cb)(struct switchdev_obj *obj))
>> +static int mv88e6xxx_port_db_dump_one(struct mv88e6xxx_chip *chip,
>> +				      u16 fid, u16 vid, int port,
>> +				      struct switchdev_obj *obj,
>> +				      int (*cb)(struct switchdev_obj *obj))
>>  {
>>  	struct mv88e6xxx_atu_entry addr = {
>>  		.mac = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
>> @@ -2222,72 +2219,87 @@ static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_chip *chip,
>>  	do {
>>  		err = _mv88e6xxx_atu_getnext(chip, fid, &addr);
>>  		if (err)
>> -			break;
>> +			return err;
>>  
>>  		if (addr.state == GLOBAL_ATU_DATA_STATE_UNUSED)
>>  			break;
>>  
>> -		if (!addr.trunk && addr.portv_trunkid & BIT(port)) {
>> -			bool is_static = addr.state ==
>> -				(is_multicast_ether_addr(addr.mac) ?
>> -				 GLOBAL_ATU_DATA_STATE_MC_STATIC :
>> -				 GLOBAL_ATU_DATA_STATE_UC_STATIC);
>> +		if (addr.trunk || (addr.portv_trunkid & BIT(port)) == 0)
>> +			continue;
>> +
>> +		if (obj->id == SWITCHDEV_OBJ_ID_PORT_FDB) {
>> +			struct switchdev_obj_port_fdb *fdb;
>>  
>> +			if (!is_unicast_ether_addr(addr.mac))
>> +				continue;
>> +
>> +			fdb = SWITCHDEV_OBJ_PORT_FDB(obj);
>>  			fdb->vid = vid;
>>  			ether_addr_copy(fdb->addr, addr.mac);
>> -			fdb->ndm_state = is_static ? NUD_NOARP : NUD_REACHABLE;
>> -
>> -			err = cb(&fdb->obj);
>> -			if (err)
>> -				break;
>> +			if (addr.state == GLOBAL_ATU_DATA_STATE_UC_STATIC)
>> +				fdb->ndm_state = NUD_NOARP;
>> +			else
>> +				fdb->ndm_state = NUD_REACHABLE;
>>  		}
>> +
>> +		err = cb(obj);
>> +		if (err)
>> +			return err;
>>  	} while (!is_broadcast_ether_addr(addr.mac));
>
> Humm, maybe i'm reading this patch wrong....
>
> This function is called mv88e6xxx_port_db_dump_one(). But i don't see
> a way out of the while loop, after dumping one. It seems to dump the
> whole table until it reaches the end marker, which is the MAC
> broadcast address.
>
> Should we rename this function, drop the _one?

No. mv88e6xxx_port_db_dump already exists, this is the function called
mv88e6xxx_port_db_dump_one multiple time. Here, _one refers to an FID
(forwarding database). 88E6352 have 4096 FIDs.

The way to dump a single FID is to run the ATU GetNext operation
starting with ff:ff:ff:ff:ff:ff, until that same address is reached
again. This is what mv88e6xxx_port_db_dump_one does.

mv88e6xxx_port_db_dump first dump the port's assigned FID (i.e. VID 0),
then iterate on active VLANs to get and dump their FIDs.

Thanks,

        Vivien

[toc] | [prev] | [next] | [standalone]

#1473585 — Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic

> No. mv88e6xxx_port_db_dump already exists, this is the function called
> mv88e6xxx_port_db_dump_one multiple time. Here, _one refers to an FID
> (forwarding database). 88E6352 have 4096 FIDs.

So maybe s/_one/_fid/ ?

   Andrew

[toc] | [prev] | [next] | [standalone]

#1473600 — Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: make switchdev DB ops generic

Andrew Lunn <andrew@lunn.ch> writes:

>> No. mv88e6xxx_port_db_dump already exists, this is the function called
>> mv88e6xxx_port_db_dump_one multiple time. Here, _one refers to an FID
>> (forwarding database). 88E6352 have 4096 FIDs.
>
> So maybe s/_one/_fid/ ?

If I have to respin this series, sure.

Thanks Andrew,

       Vivien

[toc] | [prev] | [next] | [standalone]

#1472107 — [PATCH net-next 1/3] net: dsa: add MDB support

Add SWITCHDEV_OBJ_ID_PORT_MDB support to the DSA layer.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 Documentation/networking/dsa/dsa.txt | 23 +++++++++++++++
 include/net/dsa.h                    | 16 +++++++++++
 net/dsa/slave.c                      | 55 ++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/Documentation/networking/dsa/dsa.txt b/Documentation/networking/dsa/dsa.txt
index 44ed453..6db7bc8 100644
--- a/Documentation/networking/dsa/dsa.txt
+++ b/Documentation/networking/dsa/dsa.txt
@@ -584,6 +584,29 @@ of DSA, would be the its port-based VLAN, used by the associated bridge device.
   function that the driver has to call for each MAC address known to be behind
   the given port. A switchdev object is used to carry the VID and FDB info.
 
+- port_mdb_prepare: bridge layer function invoked when the bridge prepares the
+  installation of a multicast group database entry. If the operation is not
+  supported, this function should return -EOPNOTSUPP to inform the bridge code
+  to fallback to a software implementation. No hardware setup must be done in
+  this function. See port_fdb_add for this and details.
+
+- port_mdb_add: bridge layer function invoked when the bridge wants to install 
+  a multicast group database entry, the switch hardware should be programmed 
+  with the specified address in the specified VLAN ID in the forwarding database 
+  associated with this VLAN ID.
+
+Note: VLAN ID 0 corresponds to the port private database, which, in the context
+of DSA, would be the its port-based VLAN, used by the associated bridge device.
+
+- port_mdb_del: bridge layer function invoked when the bridge wants to remove a
+  multicast group database entry, the switch hardware should be programmed to 
+  delete the specified MAC address from the specified VLAN ID if it was mapped 
+  into this port forwarding database.
+
+- port_mdb_dump: bridge layer function invoked with a switchdev callback
+  function that the driver has to call for each MAC address known to be behind
+  the given port. A switchdev object is used to carry the VID and MDB info.
+
 TODO
 ====
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2ebeba4..39f90c4 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -234,6 +234,7 @@ static inline u8 dsa_upstream_port(struct dsa_switch *ds)
 struct switchdev_trans;
 struct switchdev_obj;
 struct switchdev_obj_port_fdb;
+struct switchdev_obj_port_mdb;
 struct switchdev_obj_port_vlan;
 
 struct dsa_switch_ops {
@@ -369,6 +370,21 @@ struct dsa_switch_ops {
 	int	(*port_fdb_dump)(struct dsa_switch *ds, int port,
 				 struct switchdev_obj_port_fdb *fdb,
 				 int (*cb)(struct switchdev_obj *obj));
+
+	/*
+	 * Multicast group database
+	 */
+	int	(*port_mdb_prepare)(struct dsa_switch *ds, int port,
+				    const struct switchdev_obj_port_mdb *mdb,
+				    struct switchdev_trans *trans);
+	void	(*port_mdb_add)(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb,
+				struct switchdev_trans *trans);
+	int	(*port_mdb_del)(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb);
+	int	(*port_mdb_dump)(struct dsa_switch *ds, int port,
+				 struct switchdev_obj_port_mdb *mdb,
+				 int (*cb)(struct switchdev_obj *obj));
 };
 
 void register_switch_driver(struct dsa_switch_ops *type);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9f6c2a2..9ecbe78 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -290,6 +290,50 @@ static int dsa_slave_port_fdb_dump(struct net_device *dev,
 	return -EOPNOTSUPP;
 }
 
+static int dsa_slave_port_mdb_add(struct net_device *dev,
+				  const struct switchdev_obj_port_mdb *mdb,
+				  struct switchdev_trans *trans)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	if (switchdev_trans_ph_prepare(trans)) {
+		if (!ds->ops->port_mdb_prepare || !ds->ops->port_mdb_add)
+			return -EOPNOTSUPP;
+
+		return ds->ops->port_mdb_prepare(ds, p->port, mdb, trans);
+	}
+
+	ds->ops->port_mdb_add(ds, p->port, mdb, trans);
+
+	return 0;
+}
+
+static int dsa_slave_port_mdb_del(struct net_device *dev,
+				  const struct switchdev_obj_port_mdb *mdb)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	if (ds->ops->port_mdb_del)
+		return ds->ops->port_mdb_del(ds, p->port, mdb);
+
+	return -EOPNOTSUPP;
+}
+
+static int dsa_slave_port_mdb_dump(struct net_device *dev,
+				   struct switchdev_obj_port_mdb *mdb,
+				   switchdev_obj_dump_cb_t *cb)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	if (ds->ops->port_mdb_dump)
+		return ds->ops->port_mdb_dump(ds, p->port, mdb, cb);
+
+	return -EOPNOTSUPP;
+}
+
 static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
@@ -412,6 +456,10 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
 					     SWITCHDEV_OBJ_PORT_FDB(obj),
 					     trans);
 		break;
+	case SWITCHDEV_OBJ_ID_PORT_MDB:
+		err = dsa_slave_port_mdb_add(dev, SWITCHDEV_OBJ_PORT_MDB(obj),
+					     trans);
+		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = dsa_slave_port_vlan_add(dev,
 					      SWITCHDEV_OBJ_PORT_VLAN(obj),
@@ -435,6 +483,9 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
 		err = dsa_slave_port_fdb_del(dev,
 					     SWITCHDEV_OBJ_PORT_FDB(obj));
 		break;
+	case SWITCHDEV_OBJ_ID_PORT_MDB:
+		err = dsa_slave_port_mdb_del(dev, SWITCHDEV_OBJ_PORT_MDB(obj));
+		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = dsa_slave_port_vlan_del(dev,
 					      SWITCHDEV_OBJ_PORT_VLAN(obj));
@@ -459,6 +510,10 @@ static int dsa_slave_port_obj_dump(struct net_device *dev,
 					      SWITCHDEV_OBJ_PORT_FDB(obj),
 					      cb);
 		break;
+	case SWITCHDEV_OBJ_ID_PORT_MDB:
+		err = dsa_slave_port_mdb_dump(dev, SWITCHDEV_OBJ_PORT_MDB(obj),
+					      cb);
+		break;
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = dsa_slave_port_vlan_dump(dev,
 					       SWITCHDEV_OBJ_PORT_VLAN(obj),
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1473463 — Re: [PATCH net-next 1/3] net: dsa: add MDB support

On Mon, Aug 29, 2016 at 04:32:44PM -0400, Vivien Didelot wrote:
> Add SWITCHDEV_OBJ_ID_PORT_MDB support to the DSA layer.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  Documentation/networking/dsa/dsa.txt | 23 +++++++++++++++
>  include/net/dsa.h                    | 16 +++++++++++
>  net/dsa/slave.c                      | 55 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 94 insertions(+)
> 
> diff --git a/Documentation/networking/dsa/dsa.txt b/Documentation/networking/dsa/dsa.txt
> index 44ed453..6db7bc8 100644
> --- a/Documentation/networking/dsa/dsa.txt
> +++ b/Documentation/networking/dsa/dsa.txt
> @@ -584,6 +584,29 @@ of DSA, would be the its port-based VLAN, used by the associated bridge device.
>    function that the driver has to call for each MAC address known to be behind
>    the given port. A switchdev object is used to carry the VID and FDB info.
>  
> +- port_mdb_prepare: bridge layer function invoked when the bridge prepares the
> +  installation of a multicast group database entry.

Hi Vivien

Terminology question. This function is used to add a multicast MAC
address to the switch tables. I've always considered a multicast group
as an IP layer thing. But this documentation is also considering a
multicast group to be a layer 2 thing.

Does the bridge code use multicast group when referring to L2?

     Thanks
	Andrew

[toc] | [prev] | [next] | [standalone]

#1473539 — Re: [PATCH net-next 1/3] net: dsa: add MDB support

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Mon, Aug 29, 2016 at 04:32:44PM -0400, Vivien Didelot wrote:
>> Add SWITCHDEV_OBJ_ID_PORT_MDB support to the DSA layer.
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>  Documentation/networking/dsa/dsa.txt | 23 +++++++++++++++
>>  include/net/dsa.h                    | 16 +++++++++++
>>  net/dsa/slave.c                      | 55 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 94 insertions(+)
>> 
>> diff --git a/Documentation/networking/dsa/dsa.txt b/Documentation/networking/dsa/dsa.txt
>> index 44ed453..6db7bc8 100644
>> --- a/Documentation/networking/dsa/dsa.txt
>> +++ b/Documentation/networking/dsa/dsa.txt
>> @@ -584,6 +584,29 @@ of DSA, would be the its port-based VLAN, used by the associated bridge device.
>>    function that the driver has to call for each MAC address known to be behind
>>    the given port. A switchdev object is used to carry the VID and FDB info.
>>  
>> +- port_mdb_prepare: bridge layer function invoked when the bridge prepares the
>> +  installation of a multicast group database entry.
>
> Terminology question. This function is used to add a multicast MAC
> address to the switch tables. I've always considered a multicast group
> as an IP layer thing. But this documentation is also considering a
> multicast group to be a layer 2 thing.
>
> Does the bridge code use multicast group when referring to L2?

The switchdev MDB attribute is very similary to the FDB attribute. It
contains the VLAN ID and the Ethernet MAC address of the group.

Thanks,

        Vivien

[toc] | [prev] | [next] | [standalone]

#1473549 — Re: [PATCH net-next 1/3] net: dsa: add MDB support

> > Does the bridge code use multicast group when referring to L2?
> 
> The switchdev MDB attribute is very similary to the FDB attribute. It
> contains the VLAN ID and the Ethernet MAC address of the group.

Hi Vivien

I'm just trying to avoid the use of 'multicast group' for a L2 entity,
unless it is already widespread used so in the bridge code. If the
bridge code does consider L2 a 'multciast group', fine, lets document
it so. But if the bridge code only uses 'multciast group' to mean L3,
we should not use it here for L2.

       Andrew

[toc] | [prev] | [next] | [standalone]

#1473579 — Re: [PATCH net-next 1/3] net: dsa: add MDB support

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> > Does the bridge code use multicast group when referring to L2?
>> 
>> The switchdev MDB attribute is very similary to the FDB attribute. It
>> contains the VLAN ID and the Ethernet MAC address of the group.
>
> I'm just trying to avoid the use of 'multicast group' for a L2 entity,
> unless it is already widespread used so in the bridge code. If the
> bridge code does consider L2 a 'multciast group', fine, lets document
> it so. But if the bridge code only uses 'multciast group' to mean L3,
> we should not use it here for L2.

I am not quite sure about that. I can see that SWITCHDEV_OBJ_ID_PORT_MDB
is only crafted in net/bridge/br_mdb.c:__br_mdb_notify().

What should we do if the bridge code consider a multicast group as L3?

Thanks,

        Vivien

[toc] | [prev] | [next] | [standalone]

#1473592 — Re: [PATCH net-next 1/3] net: dsa: add MDB support

> What should we do if the bridge code consider a multicast group as L3?

Hi Vivien

Just drop the word 'group' from the Documentation. And there appears
to be one comment in the code which should be changed.

   Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web