On 3/15/23 16:18, Marcelo Ricardo Leitner wrote: > On Wed, Mar 15, 2023 at 09:33:09AM +0100, Simon Horman wrote: >> On Tue, Mar 14, 2023 at 06:49:25PM +0100, Ilya Maximets wrote: >>> On 3/14/23 12:15, Simon Horman wrote: >>>> On Mon, Mar 13, 2023 at 09:34:30PM +0100, Ilya Maximets wrote: >>>>> On 3/13/23 11:27, Roi Dayan wrote: >>>>>> From: Oz Shlomo <[email protected]> >>>>>> >>>>>> Currently jumping over a output-to-port action is translated to tc >>>>>> mirror action and stolen control action. >>>>>> However, the tc control action is not propagated to the hw offload >>>>>> action, >>>>>> thus the hardware action will mirror the packet and continue to the next >>>>>> action. >>>>> >>>>> This sounds like a bug in the hardware offloading in the kernel/driver. >>>>> Shouldn't this be fixed in the kernel? >>>> >>>> Hi Ilya, >>>> >>>> I think what we are seeing is a mismatch between how >>>> OVS translates flows to TC, and how TC actually works (since forever). >>>> >>>> It seems to me that fixing the translation, as this patch does, >>>> is the right approach. But I could be missing something (often the case). >>> >>> I'm not a TC expert, but the description sounded like the driver >>> or some other code in the kernel is ignoring the TC_ACT_STOLEN. >>> If that's correct, that must be a bug in the kernel. > > That's correct, but it's more like a tc feature that hasn't been > implemented in the HWOL side of it and that shouldn't have been used > here. Or a gap, or so, but still. It's not a small piece of code. Tc > is super flexible and that flexibility is not there in the driver > sometimes.
Sure, but "something is complex" is not really an excuse for that something to not operate correctly. :) Unfortunately, it's a common theme with hardware offload in TC incorrectly accepting and misinterpreting what shouldn't have been accepted in the first place, or ignoring things that shouldn't have been ignored. And this issue in TC should have been addressed when the hardware offload parts of TC were first introduced. If not by implementing the "missing feature", then, at least, by making sure that rules that are using this feature are not going to hardware. Either way it's an internal TC bug and not OVS'es fault. Again, to be clear, I'm OK with fixing it on OVS side by not using broken API, but the API should be fixed in the kernel, regardless. Otherwise, someone else will hit that issue. It may even be us again in the future. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
