On 22 Feb 2023, at 13:46, Simon Horman wrote:
> On Wed, Feb 22, 2023 at 12:30:17PM +0200, Roi Dayan via dev 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 <r...@nvidia.com>
>
> I think this needs an ack from Eelco (CCed).
>
> But it looks good to me.
>
> Reviewed-by: Simon Horman <simon.hor...@corigine.com>
The changes look good to me. Will it be worth adding a test case?
Acked-by: Eelco Chaudron <echau...@redhat.com>
>> ---
>> 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);
>> }
>>
>> --
>> 2.38.0
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
