lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Sep 2021 02:30:16 +0000 From: Xiaoliang Yang <xiaoliang.yang_1@....com> To: Vladimir Oltean <vladimir.oltean@....com> CC: "davem@...emloft.net" <davem@...emloft.net>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "allan.nielsen@...rochip.com" <allan.nielsen@...rochip.com>, "joergen.andreasen@...rochip.com" <joergen.andreasen@...rochip.com>, "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>, "vinicius.gomes@...el.com" <vinicius.gomes@...el.com>, "michael.chan@...adcom.com" <michael.chan@...adcom.com>, "vishal@...lsio.com" <vishal@...lsio.com>, "saeedm@...lanox.com" <saeedm@...lanox.com>, "jiri@...lanox.com" <jiri@...lanox.com>, "idosch@...lanox.com" <idosch@...lanox.com>, "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>, "kuba@...nel.org" <kuba@...nel.org>, Po Liu <po.liu@....com>, Leo Li <leoyang.li@....com>, "f.fainelli@...il.com" <f.fainelli@...il.com>, "andrew@...n.ch" <andrew@...n.ch>, "vivien.didelot@...il.com" <vivien.didelot@...il.com>, Claudiu Manoil <claudiu.manoil@....com> Subject: RE: [PATCH v4 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959 Hi Vladimir, On Wed, Sep 22, 2021 at 12:47:59 +0000, Vladimir Oltean 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? Maybe we need to use ocelot_mact_learn() instead of ocelot_mact_write() after setting SFID in StreamData. I think this can avoid writing a wrong entry. Regards, Xiaoliang
Powered by blists - more mailing lists