On 17 Feb 2022, at 16:07, Ilya Maximets wrote:
> On 2/17/22 13:57, Ilya Maximets wrote:
>> On 1/31/22 11:34, Eelco Chaudron wrote:
>>> Verify that the returned ifindex by netdev_get_ifindex() is valid.
>>> This might not be the case in the ERSPAN port scenario, which can
>>> not be offloaded.
>>
>> Why exactly we can't get ifindex of the kernel interface?
>
> Hmm. I see that netdev_vport_tunnel_register() doesn't register
> the .get_ifindex callback for erspan and some other tunnel types,
> but I'm not sure why...
Same here. I could not think of a reason. But did not want to break anything by
adding it.
>>>
>>> Signed-off-by: Eelco Chaudron <[email protected]>
>>> ---
>>> lib/netdev-offload-tc.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>>> index 9845e8d3f..8135441c6 100644
>>> --- a/lib/netdev-offload-tc.c
>>> +++ b/lib/netdev-offload-tc.c
>>> @@ -1842,6 +1842,15 @@ netdev_tc_flow_put(struct netdev *netdev, struct
>>> match *match,
>>> return ENODEV;
>>> }
>>> action->out.ifindex_out = netdev_get_ifindex(outdev);
>>> +
>>> + if (action->out.ifindex_out < 0) {
>>> + VLOG_DBG_RL(&rl,
>>> + "Can't find ifindex for output port %s, error
>>> %d",
>>> + netdev_get_name(outdev),
>>> action->out.ifindex_out);
>>> +
>>
>> We're leaking netdev reference here.
Good catch!
>> Shouldn't we check netdev_flow_api_equals(netdev, outdev) here instead?
>> This way we will be sure that flow API was successfully initialized for
>> the outdev and we're not mixing different offload providers.
Decided to add it, and keep the index check also. So it will look something
like:
@@ -1841,7 +1841,24 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
*match,
VLOG_DBG_RL(&rl, "Can't find netdev for output port %d", port);
return ENODEV;
}
+
+ if (!netdev_flow_api_equals(netdev, outdev)) {
+ VLOG_DBG_RL(&rl,
+ "Egress port %s is using a different flow API "
+ "provider", netdev_get_name(outdev));
+ netdev_close(outdev);
+ return EOPNOTSUPP;
+ }
+
action->out.ifindex_out = netdev_get_ifindex(outdev);
+ if (action->out.ifindex_out < 0) {
+ VLOG_DBG_RL(&rl,
+ "Can't find ifindex for output port %s, error %d",
+ netdev_get_name(outdev), action->out.ifindex_out);
+ netdev_close(outdev);
+ return -action->out.ifindex_out;
+ }
+
>>> + return -action->out.ifindex_out;
>>> + }
>>> +
>>> action->out.ingress =
>>> is_internal_port(netdev_get_type(outdev));
>>> action->type = TC_ACT_OUTPUT;
>>> flower.action_count++;
>>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
