kernel/linux.git/drivers/net/ethernet/mscc, branch v5.10.259

net: mscc: ocelot: fix VCAP filters not matching on MAC with "protocol 802.1Q"

2023-02-15T16:22:24+00:00

[ Upstream commit f964f8399df29d3e3ced77177cf35131cd2491bf ] Alternative short title: don't instruct the hardware to match on EtherType with "protocol 802.1Q" flower filters. It doesn't work for the reasons detailed below. With a command such as the following: tc filter add dev $swp1 ingress chain $(IS1 2) pref 3 \ protocol 802.1Q flower skip_sw vlan_id 200 src_mac $h1_mac \ action vlan modify id 300 \ action goto chain $(IS2 0 0) the created filter is set by ocelot_flower_parse_key() to be of type OCELOT_VCAP_KEY_ETYPE, and etype is set to {value=0x8100, mask=0xffff}. This gets propagated all the way to is1_entry_set() which commits it to hardware (the VCAP_IS1_HK_ETYPE field of the key). Compare this to the case where src_mac isn't specified - the key type is OCELOT_VCAP_KEY_ANY, and is1_entry_set() doesn't populate VCAP_IS1_HK_ETYPE. The problem is that for VLAN-tagged frames, the hardware interprets the ETYPE field as holding the encapsulated VLAN protocol. So the above filter will only match those packets which have an encapsulated protocol of 0x8100, rather than all packets with VLAN ID 200 and the given src_mac. The reason why this is allowed to occur is because, although we have a block of code in ocelot_flower_parse_key() which sets "match_protocol" to false when VLAN keys are present, that code executes too late. There is another block of code, which executes for Ethernet addresses, and has a "goto finished_key_parsing" and skips the VLAN header parsing. By skipping it, "match_protocol" remains with the value it was initialized with, i.e. "true", and "proto" is set to f->common.protocol, or 0x8100. The concept of ignoring some keys rather than erroring out when they are present but can't be offloaded is dubious in itself, but is present since the initial commit fe3490e6107e ("net: mscc: ocelot: Hardware ofload for tc flower filter"), and it's outside of the scope of this patch to change that. The problem was introduced when the driver started to interpret the flower filter's protocol, and populate the VCAP filter's ETYPE field based on it. To fix this, it is sufficient to move the code that parses the VLAN keys earlier than the "goto finished_key_parsing" instruction. This will ensure that if we have a flower filter with both VLAN and Ethernet address keys, it won't match on ETYPE 0x8100, because the VLAN key parsing sets "match_protocol = false". Fixes: 86b956de119c ("net: mscc: ocelot: support matching on EtherType") Signed-off-by: Vladimir Oltean Reviewed-by: Simon Horman Link: https://lore.kernel.org/r/20230205192409.1796428-1-vladimir.oltean@nxp.com Signed-off-by: Paolo Abeni Signed-off-by: Sasha Levin

net: mscc: ocelot: allow unregistered IP multicast flooding

2022-07-02T14:39:25+00:00

Flooding of unregistered IP multicast has been broken (both to other switch ports and to the CPU) since the ocelot driver introduction, and up until commit 4cf35a2b627a ("net: mscc: ocelot: fix broken IP multicast flooding"), a bug fix for commit 421741ea5672 ("net: mscc: ocelot: offload bridge port flags to device") from v5.12. The driver used to set PGID_MCIPV4 and PGID_MCIPV6 to the empty port mask (0), which made unregistered IPv4/IPv6 multicast go nowhere, and without ever modifying that port mask at runtime. The expectation is that such packets are treated as broadcast, and flooded according to the forwarding domain (to the CPU if the port is standalone, or to the CPU and other bridged ports, if under a bridge). Since the aforementioned commit, the limitation has been lifted by responding to SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS events emitted by the bridge. As for host flooding, DSA synthesizes another call to ocelot_port_bridge_flags() on the NPI port which ensures that the CPU gets the unregistered multicast traffic it might need, for example for smcroute to work between standalone ports. But between v4.18 and v5.12, IP multicast flooding has remained unfixed. Delete the inexplicable premature optimization of clearing PGID_MCIPV4 and PGID_MCIPV6 as part of the init sequence, and allow unregistered IP multicast to be flooded freely according to the forwarding domain established by PGID_SRC, by explicitly programming PGID_MCIPV4 and PGID_MCIPV6 towards all physical ports plus the CPU port module. Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support") Cc: stable@kernel.org Signed-off-by: Vladimir Oltean Signed-off-by: Greg Kroah-Hartman

net: mscc: ocelot: avoid corrupting hardware counters when moving VCAP filters

2022-05-18T08:23:43+00:00

[ Upstream commit 93a8417088ea570b5721d2b526337a2d3aed9fa3 ] Given the following order of operations: (1) we add filter A using tc-flower (2) we send a packet that matches it (3) we read the filter's statistics to find a hit count of 1 (4) we add a second filter B with a higher preference than A, and A moves one position to the right to make room in the TCAM for it (5) we send another packet, and this matches the second filter B (6) we read the filter statistics again. When this happens, the hit count of filter A is 2 and of filter B is 1, despite a single packet having matched each filter. Furthermore, in an alternate history, reading the filter stats a second time between steps (3) and (4) makes the hit count of filter A remain at 1 after step (6), as expected. The reason why this happens has to do with the filter->stats.pkts field, which is written to hardware through the call path below: vcap_entry_set / | \ / | \ / | \ / | \ es0_entry_set is1_entry_set is2_entry_set \ | / \ | / \ | / vcap_data_set(data.counter, ...) The primary role of filter->stats.pkts is to transport the filter hit counters from the last readout all the way from vcap_entry_get() -> ocelot_vcap_filter_stats_update() -> ocelot_cls_flower_stats(). The reason why vcap_entry_set() writes it to hardware is so that the counters (saturating and having a limited bit width) are cleared after each user space readout. The writing of filter->stats.pkts to hardware during the TCAM entry movement procedure is an unintentional consequence of the code design, because the hit count isn't up to date at this point. So at step (4), when filter A is moved by ocelot_vcap_filter_add() to make room for filter B, the hardware hit count is 0 (no packet matched on it in the meantime), but filter->stats.pkts is 1, because the last readout saw the earlier packet. The movement procedure programs the old hit count back to hardware, so this creates the impression to user space that more packets have been matched than they really were. The bug can be seen when running the gact_drop_and_ok_test() from the tc_actions.sh selftest. Fix the issue by reading back the hit count to tmp->stats.pkts before migrating the VCAP filter. Sure, this is a best-effort technique, since the packets that hit the rule between vcap_entry_get() and vcap_entry_set() won't be counted, but at least it allows the counters to be reliably used for selftests where the traffic is under control. The vcap_entry_get() name is a bit unintuitive, but it only reads back the counter portion of the TCAM entry, not the entire entry. The index from which we retrieve the counter is also a bit unintuitive (i - 1 during add, i + 1 during del), but this is the way in which TCAM entry movement works. The "entry index" isn't a stored integer for a TCAM filter, instead it is dynamically computed by ocelot_vcap_block_get_filter_index() based on the entry's position in the &block->rules list. That position (as well as block->count) is automatically updated by ocelot_vcap_filter_add_to_block() on add, and by ocelot_vcap_block_remove_filter() on del. So "i" is the new filter index, and "i - 1" or "i + 1" respectively are the old addresses of that TCAM entry (we only support installing/deleting one filter at a time). Fixes: b596229448dd ("net: mscc: ocelot: Add support for tcam") Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: restrict tc-trap actions to VCAP IS2 lookup 0

2022-05-18T08:23:42+00:00

[ Upstream commit 477d2b91623e682e9a8126ea92acb8f684969cc7 ] Once the CPU port was added to the destination port mask of a packet, it can never be cleared, so even packets marked as dropped by the MASK_MODE of a VCAP IS2 filter will still reach it. This is why we need the OCELOT_POLICER_DISCARD to "kill dropped packets dead" and make software stop seeing them. We disallow policer rules from being put on any other chain than the one for the first lookup, but we don't do this for "drop" rules, although we should. This change is merely ascertaining that the rules dont't (completely) work and letting the user know. The blamed commit is the one that introduced the multi-chain architecture in ocelot. Prior to that, we should have always offloaded the filters to VCAP IS2 lookup 0, where they did work. Fixes: 1397a2eb52e2 ("net: mscc: ocelot: create TCAM skeleton from tc filter chains") Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: fix VCAP IS2 filters matching on both lookups

2022-05-18T08:23:42+00:00

[ Upstream commit 6741e11880003e35802d78cc58035057934f4dab ] The VCAP IS2 TCAM is looked up twice per packet, and each filter can be configured to only match during the first, second lookup, or both, or none. The blamed commit wrote the code for making VCAP IS2 filters match only on the given lookup. But right below that code, there was another line that explicitly made the lookup a "don't care", and this is overwriting the lookup we've selected. So the code had no effect. Some of the more noticeable effects of having filters match on both lookups: - in "tc -s filter show dev swp0 ingress", we see each packet matching a VCAP IS2 filter counted twice. This throws off scripts such as tools/testing/selftests/net/forwarding/tc_actions.sh and makes them fail. - a "tc-drop" action offloaded to VCAP IS2 needs a policer as well, because once the CPU port becomes a member of the destination port mask of a packet, nothing removes it, not even a PERMIT/DENY mask mode with a port mask of 0. But VCAP IS2 rules with the POLICE_ENA bit in the action vector can only appear in the first lookup. What happens when a filter matches both lookups is that the action vector is combined, and this makes the POLICE_ENA bit ineffective, since the last lookup in which it has appeared is the second one. In other words, "tc-drop" actions do not drop packets for the CPU port, dropped packets are still seen by software unless there was an FDB entry that directed those packets to some other place different from the CPU. The last bit used to work, because in the initial commit b596229448dd ("net: mscc: ocelot: Add support for tcam"), we were writing the FIRST field of the VCAP IS2 half key with a 1, not with a "don't care". The change to "don't care" was made inadvertently by me in commit c1c3993edb7c ("net: mscc: ocelot: generalize existing code for VCAP"), which I just realized, and which needs a separate fix from this one, for "stable" kernels that lack the commit blamed below. Fixes: 226e9cd82a96 ("net: mscc: ocelot: only install TCAM entries into a specific lookup and PAG") Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: fix last VCAP IS1/IS2 filter persisting in hardware when deleted

2022-05-18T08:23:42+00:00

[ Upstream commit 16bbebd35629c93a8c68c6d8d28557e100bcee73 ] ocelot_vcap_filter_del() works by moving the next filters over the current one, and then deleting the last filter by calling vcap_entry_set() with a del_filter which was specially created by memsetting its memory to zeroes. vcap_entry_set() then programs this to the TCAM and action RAM via the cache registers. The problem is that vcap_entry_set() is a dispatch function which looks at del_filter->block_id. But since del_filter is zeroized memory, the block_id is 0, or otherwise said, VCAP_ES0. So practically, what we do is delete the entry at the same TCAM index from VCAP ES0 instead of IS1 or IS2. The code was not always like this. vcap_entry_set() used to simply be is2_entry_set(), and then, the logic used to work. Restore the functionality by populating the block_id of the del_filter based on the VCAP block of the filter that we're deleting. This makes vcap_entry_set() know what to do. Fixes: 1397a2eb52e2 ("net: mscc: ocelot: create TCAM skeleton from tc filter chains") Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: fix backwards compatibility with single-chain tc-flower offload

2022-03-23T08:13:28+00:00

[ Upstream commit 8e0341aefcc9133f3f48683873284b169581315b ] ACL rules can be offloaded to VCAP IS2 either through chain 0, or, since the blamed commit, through a chain index whose number encodes a specific PAG (Policy Action Group) and lookup number. The chain number is translated through ocelot_chain_to_pag() into a PAG, and through ocelot_chain_to_lookup() into a lookup number. The problem with the blamed commit is that the above 2 functions don't have special treatment for chain 0. So ocelot_chain_to_pag(0) returns filter->pag = 224, which is in fact -32, but the "pag" field is an u8. So we end up programming the hardware with VCAP IS2 entries having a PAG of 224. But the way in which the PAG works is that it defines a subset of VCAP IS2 filters which should match on a packet. The default PAG is 0, and previous VCAP IS1 rules (which we offload using 'goto') can modify it. So basically, we are installing filters with a PAG on which no packet will ever match. This is the hardware equivalent of adding filters to a chain which has no 'goto' to it. Restore the previous functionality by making ACL filters offloaded to chain 0 go to PAG 0 and lookup number 0. The choice of PAG is clearly correct, but the choice of lookup number isn't "as before" (which was to leave the lookup a "don't care"). However, lookup 0 should be fine, since even though there are ACL actions (policers) which have a requirement to be used in a specific lookup, that lookup is 0. Fixes: 226e9cd82a96 ("net: mscc: ocelot: only install TCAM entries into a specific lookup and PAG") Signed-off-by: Vladimir Oltean Link: https://lore.kernel.org/r/20220316192117.2568261-1-vladimir.oltean@nxp.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: fix mutex lock error during ethtool stats read

2022-02-16T11:54:27+00:00

[ Upstream commit 7fbf6795d127a3b1bb39b0e42579904cf6db1624 ] An ongoing workqueue populates the stats buffer. At the same time, a user might query the statistics. While writing to the buffer is mutex-locked, reading from the buffer wasn't. This could lead to buggy reads by ethtool. This patch fixes the former blamed commit, but the bug was introduced in the latter. Signed-off-by: Colin Foster Fixes: 1e1caa9735f90 ("ocelot: Clean up stats update deferred work") Fixes: a556c76adc052 ("net: mscc: Add initial Ocelot switch support") Reported-by: Vladimir Oltean Reviewed-by: Vladimir Oltean Link: https://lore.kernel.org/all/20220210150451.416845-2-colin.foster@in-advantage.com/ Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: mscc: ocelot: fix using match before it is set

2022-01-27T09:54:35+00:00

commit baa59504c1cd0cca7d41954a45ee0b3dc78e41a0 upstream. Clang static analysis reports this issue ocelot_flower.c:563:8: warning: 1st function call argument is an uninitialized value !is_zero_ether_addr(match.mask->dst)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The variable match is used before it is set. So move the block. Fixes: 75944fda1dfe ("net: mscc: ocelot: offload ingress skbedit and vlan actions to VCAP IS1") Signed-off-by: Tom Rix Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman

net: mscc: ocelot: correctly report the timestamping RX filters in ethtool

2021-12-01T08:19:08+00:00

[ Upstream commit c49a35eedfef08bffd46b53c25dbf9d6016a86ff ] The driver doesn't support RX timestamping for non-PTP packets, but it declares that it does. Restrict the reported RX filters to PTP v2 over L2 and over L4. Fixes: 4e3b0468e6d7 ("net: mscc: PTP Hardware Clock (PHC) support") Signed-off-by: Vladimir Oltean Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin