On 30.07.2019 13:30, Ilya Maximets wrote: > On 30.07.2019 13:23, Eli Britstein wrote: >> >> On 7/30/2019 1:10 PM, Ilya Maximets wrote: >>> On 03.07.2019 7:58, Eli Britstein wrote: >>>> Ethernet type 0x1234 is used for testing and not being offloadable. For >>>> testing offloadable features, log about using this value. >>>> >>>> Signed-off-by: Eli Britstein <el...@mellanox.com> >>>> Acked-by: Roi Dayan <r...@mellanox.com> >>>> Signed-off-by: Eli Britstein <el...@mellanox.com> >>>> Acked-by: Ben Pfaff <b...@ovn.org> >>>> --- >>>> lib/dpif-netlink.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >>>> index ba80a0079..a0d51ae61 100644 >>>> --- a/lib/dpif-netlink.c >>>> +++ b/lib/dpif-netlink.c >>>> @@ -2007,6 +2007,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct >>>> dpif_flow_put *put) >>>> >>>> /* When we try to install a dummy flow from a probed feature. */ >>>> if (match.flow.dl_type == htons(0x1234)) { >>>> + VLOG_INFO_RL(&rl, "eth 0x1234 is special and not offloadable"); >>>> return EOPNOTSUPP; >>>> } >>> But what is the purpose of this patch? What is the use case? >> RH wanted to test that dl_type is offloaded. Coincidentally, they used >> 0x1234, and it was not offloaded, and they didn't understand why, and >> suggested a log message. > > I'll take a closer look, but it seems that we just need to remove this > 'if' statement and allow oflloading.
'dpif_probe_feature' always has DPIF_FP_PROBE flag set. Other probing code uses dpif_execute() which uses DPIF_OP_EXECUTE, hence never calls parse_flow_put(). So, this 'if' statement is wrong and should be deleted as it only forbids offloading of the real legitimate flows with dl_type 0x1234. Dummy flows never reaches this code. > >>> >>> Actually, it looks like we need to just remove above 'if' statement >>> entirely. Just a few lines above there is a check if we are probing >>> or installing real flow: >>> >>> if (put->flags & DPIF_FP_PROBE) { >>> return EOPNOTSUPP; >>> } >>> >>> So, we will never get there while probing. But why we're restricting >>> this flow type from being offloaded? 'netdev_flow_put' will refuse >>> to offload if it doesn't know that flow type, but this shouldn't be >>> done here. >>> >>> In case we have a dummy flow without DPIF_FP_PROBE flag set, we need >>> to fix upper layers. Is it the case? >> >> I didn't look into it why we restrict it and if there is a real reason >> why in this layer. You may be right, but I don't know. >> >>> >>> Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev