On 11/6/25 5:39 PM, Aaron Conole wrote: > Ilya Maximets <[email protected]> writes: > >> If someone creates a lot of meters (>1017) in the kernel datapath >> and then re-starts ovs-vswitchd, OVS fails to probe meter support >> and refuses to install any meters afterwards: >> >> dpif_netlink|INFO|The kernel module has a broken meter implementation. >> >> The reason is that probing for broken meters relies on creating >> two meters with high meter IDs. Inside the kernel, however, the >> meter table is not a real hash table since v5.10 and commit: >> >> c7c4c44c9a95 ("net: openvswitch: expand the meters supported number") >> >> Instead, it's just an array with meter IDs mapped to indexes with a >> simple modulo operation. This array can expand, but only when it is >> full. There is no handling of collisions, so if the meter at >> ID % size is not the right one, then the lookup just fails. This is >> fine as long as userspace creates meters with densely packed IDs, >> which is the case for ovs-vswitchd... except for probing. >> >> While probing, we attempt to create meters 54545401 and 54545402. >> Without expanding the table size, these map onto 1017 and 1018. >> So, creation of these meters fails if there are already meters with >> IDs 1017 or 1018. At this point OVS declares meter implementation >> broken. > > Just a comment that it shouldn't actually matter what the meter IDs are, > right? For example, you change from the very high values, but it > actually doesn't affect whether the meter ID check is working. The big > issue is that we're not really allowing for the case that a meter > already exists.
Yeah, that's true. It will fail on any collision. The text above just highlights why we have collisions at all even if we use such very large numbers. > >> Ideally, we should make the "hash" table in the kernel handle >> collisions and otherwise be a more or less proper hash table. But >> we can also improve probing in userspace and avoid this issue by >> choosing lower numbered meter IDs and trying to get them from the >> kernel first before trying to create. Choosing high values at the >> top of the 0-1023 range, so they are guaranteed to fit into the >> minimal size table in the kernel (1024). If one of these already >> exists and has a proper ID, then meters are likely working fine >> and we don't need to install new ones for probing. If these meters >> are not in the kernel, then we can try to create and check. >> >> This logic should work fine for older or future kernels with a >> proper hash table as well. >> >> There is no Fixes tag here as the check was correct at the moment >> it was introduced. It's the kernel change that broke it. >> >> Reported-at: https://github.com/openvswitch/ovs-issues/issues/337 >> Signed-off-by: Ilya Maximets <[email protected]> >> --- > > Acked-by: Aaron Conole <[email protected]> > Thanks! Applied and backported down to 3.3. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
