Hello Xiaoliang,

On Wed, Sep 22, 2021 at 06:51:59PM +0800, Xiaoliang Yang wrote:

+static int vsc9959_mact_stream_set(struct ocelot *ocelot,
+				   struct felix_stream *stream,
+				   struct netlink_ext_ack *extack)
+{
+	struct ocelot_mact_entry entry;
+	u32 row, col, reg, dst_idx;
+	u8 type;
+	int ret;
+
+	/* Stream identification desn't support to add a stream with non
+	 * existent MAC (The MAC entry has not been learned in MAC table).
+	 */
+	ret = ocelot_mact_lookup(ocelot, stream->dmac, stream->vid, &row, &col);
+	if (ret) {
+		if (extack)
+			NL_SET_ERR_MSG_MOD(extack, "Stream is not learned in MAC table");
+		return -EOPNOTSUPP;
+	}
+
+	ocelot_rmw(ocelot,
+		   (stream->sfid_valid ? ANA_TABLES_STREAMDATA_SFID_VALID : 0) |
+		   ANA_TABLES_STREAMDATA_SFID(stream->sfid),
+		   ANA_TABLES_STREAMDATA_SFID_VALID |
+		   ANA_TABLES_STREAMDATA_SFID_M,
+		   ANA_TABLES_STREAMDATA);
+
+	reg = ocelot_read(ocelot, ANA_TABLES_MACACCESS);
+	dst_idx = (reg & ANA_TABLES_MACACCESS_DEST_IDX_M) >> 3;
+	type = ANA_TABLES_MACACCESS_ENTRYTYPE_X(reg);
+
+	reg = ocelot_read(ocelot, ANA_TABLES_STREAMDATA);
+	if ((ANA_TABLES_STREAMDATA_SFID_VALID |
+	     ANA_TABLES_STREAMDATA_SSID_VALID) & reg) {
+		entry.type = (type ? type : ENTRYTYPE_LOCKED);
+		stream->rsv_type = type;
+	} else {
+		entry.type = stream->rsv_type;
+	}
+
+	ether_addr_copy(entry.mac, stream->dmac);
+	entry.vid = stream->vid;
+
+	ocelot_mact_write(ocelot, dst_idx, &entry, row, col);
+
+	return 0;
+}

+static int vsc9959_stream_table_add(struct ocelot *ocelot,
+				    struct list_head *stream_list,
+				    struct felix_stream *stream,
+				    struct netlink_ext_ack *extack)
+{
+	struct felix_stream *stream_entry;
+	int ret;
+
+	stream_entry = kzalloc(sizeof(*stream_entry), GFP_KERNEL);
+	if (!stream_entry)
+		return -ENOMEM;
+
+	memcpy(stream_entry, stream, sizeof(*stream_entry));
+
+	ret = vsc9959_mact_stream_set(ocelot, stream_entry, extack);
+	if (ret) {
+		kfree(stream_entry);
+		return ret;
+	}
+
+	list_add_tail(&stream_entry->list, stream_list);
+
+	return 0;
+}

Remember this discussion we had a while ago?

| Let's take the function below.
| 
| static void ocelot_prove_mac_table_entries_can_move(struct ocelot *ocelot)
| {
| 	unsigned char mac1[ETH_ALEN] = {0x00, 0x04, 0x9f, 0x63, 0x35, 0xea};
| 	unsigned char mac2[ETH_ALEN] = {0x00, 0x04, 0x9f, 0x63, 0x35, 0xeb};
| 	int row, bucket, arbitrary_pgid = 4;
| 	int vid1 = 102;
| 	int vid2 = 103;
| 	int err;
| 
| 	err = ocelot_mact_learn(ocelot, arbitrary_pgid, mac1, vid1,
| 				ENTRYTYPE_LOCKED);
| 	if (err)
| 		return;
| 
| 	err = ocelot_mact_lookup(ocelot, mac1, vid1, &row, &bucket);
| 	if (err)
| 		return;
| 
| 	dev_info(ocelot->dev,
| 		 "Address 1 (mac %pM vid %d) is in MAC table row %d bucket %d\n",
| 		 mac1, vid1, row, bucket);
| 
| 	err = ocelot_mact_learn(ocelot, arbitrary_pgid, mac2, vid2,
| 				ENTRYTYPE_LOCKED);
| 	if (err)
| 		return;
| 
| 	err = ocelot_mact_lookup(ocelot, mac2, vid2, &row, &bucket);
| 	if (err)
| 		return;
| 
| 	dev_info(ocelot->dev,
| 		 "Address 2 (mac %pM vid %d) is in MAC table row %d bucket %d\n",
| 		 mac2, vid2, row, bucket);
| 
| 	err = ocelot_mact_lookup(ocelot, mac1, vid1, &row, &bucket);
| 	if (err)
| 		return;
| 
| 	dev_info(ocelot->dev,
| 		 "Address 1 (mac %pM vid %d) is in MAC table row %d bucket %d\n",
| 		 mac1, vid1, row, bucket);
| }
| 
| What will it print?
| 
| Address 1 (mac 00:04:9f:63:35:ea vid 102) is in MAC table row 917 bucket 0
| Address 2 (mac 00:04:9f:63:35:eb vid 103) is in MAC table row 917 bucket 0
| Address 1 (mac 00:04:9f:63:35:ea vid 102) is in MAC table row 917 bucket 1
| 
| What does this mean?
| 
| The ROW portion of a FDB entry's position within the MAC table is
| statically determined using an 11-bit hash derived from the {DMAC, VID}
| key. Within a row, there can be up to 4 buckets, each bucket holding 1
| MAC table entry.
| 
| But when the hashes of 2 addresses collide and they end up in the same
| row (as in the above example, with address 1 = "mac 00:04:9f:63:35:ea
| vid 102" and address 2 = "mac 00:04:9f:63:35:eb vid 103"), things don't
| happen quite as you might expect. Namely, the second address appears to
| be installed by the switch at the same row and bucket as the first
| address. So is the first address overwritten? No, it has been moved by
| the switch, automatically, to bucket 1.

So if the autonomous and concurrent learning of one MAC address might
move existing MAC table entries from a row to the right, then who
guarantees exactly that the {row, col} for which you are setting up the
SFID is the {row, col} that belongs to the {stream->dmac, stream->vid}
you have searched for?

Microchip people, do we need to temporarily disable hardware address
learning on all ports, and take a lock with the FDB add and delete
operations to ensure they are serialized?