On Thu, Aug 6, 2026 at 1:34 PM Jakub Kicinski <[email protected]> wrote: > > On Thu, 6 Aug 2026 13:05:49 -0700 Mina Almasry wrote: > > Yes the test fails, but it gets past the flow steering configuration > > problem. > > > > And in the code, the change looks correct to me. If flow_steer is > > False, then we don't pass the -c arg to ncdevmem. ncdevmem is written > > so that if -c is not set, 5-tuple flow steering is not possible (the > > client side port is random), so it falls back to 3-tuple flow > > steering, which doesn't work on GVE or any other driver that supports > > only 5-tuple flow steering. > > > > run_rx (devmem.check_rx test case) doesn't have this problem because > > it does `flow_steer=not hasattr(cfg, 'netns')`, but run_rx_hds has > > this problem because it forgets to override flow_steer at all. This > > patch fixes that. > > > > To be honest while looking at this there were a few weird things. Like > > I'm not sure why the python arg is named `flow_steer` (it likely > > should be 5_tuple_flow_steer), and why flow_steer setting is tied to > > hasattr(cfg, 'netns'), it should be a proprety of the driver you're > > runing on (unless all in-netns drivers will support 3-tuple flow > > steering). > > Could we clean this up and add explicit test cases for inserting > specific rule types? We want the one-sided-tuple format at Meta, > falling back silently would be a loss of signal.
Do you mean forking all the test cases so that we have check_rx_3_tuple and check_rx_5_tuple, etc? Won't that be annoying in the future if every test case needs to be duplicated twice? And potentially again if there is another deviation in driver config support specifics? The current approach is that yes we have 1 set of test cases, and they're supposed to auto-detect if 3-tuple or 5-tuple is supported and fallback silently to what the driver supports. check_rx and check_tx currently work this way. I occasionally find breakages on 5-tuple setups because they're less common and fix them, like this one[1]. To be clear this change is not intended to break 3-tuple. It's just porting the bit that makes check_rx work for check_rx_hds. Whoever added run_rx_hds probably did not have access to a 5-tuple-only driver and missed this needed change. [1] https://lore.kernel.org/netdev/aDXbNuCPNKRYYVRk@mini-arch/ -- Thanks, Mina
