Messages in this thread | | | From | Vladimir Oltean <> | Subject | Re: [PATCH v4 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959 | Date | Wed, 22 Sep 2021 12:47:59 +0000 |
| |
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?
| |