On 13/03/2023 11:01, Eelco Chaudron wrote:
>
>
> On 13 Mar 2023, at 9:38, Roi Dayan wrote:
>
>> On 22/02/2023 12:30, Roi Dayan wrote:
>>> Sometimes there is a need to clean empty chains as done in
>>> delete_chains_from_netdev(). The cited commit doesn't remove
>>> the chain completely which cause adding ingress_block later to fail.
>>> This can be reproduced with adding bond as ovs port which makes ovs
>>> use ingress_block for it.
>>> While at it add the netdev name that fails to the log.
>>>
>>> Fixes: e1e5eac5b016 ("tc: Add TCA_KIND flower to delete and get operation
>>> to avoid rtnl_lock().")
>>> Signed-off-by: Roi Dayan <[email protected]>
>>> ---
>>> lib/netdev-offload-tc.c | 7 ++++---
>>> lib/tc.c | 4 +++-
>>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>>> index 4fb9d9f2127a..9dd0aa2e2a85 100644
>>> --- a/lib/netdev-offload-tc.c
>>> +++ b/lib/netdev-offload-tc.c
>>> @@ -524,7 +524,7 @@ delete_chains_from_netdev(struct netdev *netdev, struct
>>> tcf_id *id)
>>> */
>>> HMAP_FOR_EACH_POP (chain_node, node, &map) {
>>> id->chain = chain_node->chain;
>>> - tc_del_flower_filter(id);
>>> + tc_del_filter(id, NULL);
>>> free(chain_node);
>>> }
>>> }
>>> @@ -2860,8 +2860,9 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>>> error = tc_add_del_qdisc(ifindex, true, block_id, hook);
>>>
>>> if (error && error != EEXIST) {
>>> - VLOG_INFO("failed adding ingress qdisc required for offloading:
>>> %s",
>>> - ovs_strerror(error));
>>> + VLOG_INFO("failed adding ingress qdisc required for offloading "
>>> + "on %s: %s",
>>> + netdev_get_name(netdev), ovs_strerror(error));
>>> return error;
>>> }
>>>
>>> diff --git a/lib/tc.c b/lib/tc.c
>>> index 4c07e22162e7..5c32c6f971da 100644
>>> --- a/lib/tc.c
>>> +++ b/lib/tc.c
>>> @@ -2354,7 +2354,9 @@ tc_del_filter(struct tcf_id *id, const char *kind)
>>> struct ofpbuf request;
>>>
>>> request_from_tcf_id(id, 0, RTM_DELTFILTER, NLM_F_ACK, &request);
>>> - nl_msg_put_string(&request, TCA_KIND, kind);
>>> + if (kind) {
>>> + nl_msg_put_string(&request, TCA_KIND, kind);
>>> + }
>>> return tc_transact(&request, NULL);
>>> }
>>>
>>
>> hi
>>
>> just pinging about this fix.
>
> Guess it’s waiting on your feedback on Simon’s reply:
>
> EC> The changes look good to me. Will it be worth adding a test case?
>
> SH> From my POV, yes, I think that would be nice.
> SH> Roi, do you have any thoughts on this?
>
Oh. thanks for the updates.
I missed the replies. if I'm not on the to/cc the mailing list emails are going
to
a different folder so I could catch emails when I am on to/cc better.
>> thanks,
>> Roi
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
