[ovs-dev] [PATCH] netdev-offload-tc: Flush rules on ingress block when init tc flow api

Ilya Maximets i.maximets at ovn.org
Tue Mar 3 13:15:21 UTC 2020


On 3/3/20 8:58 AM, Roi Dayan wrote:
> 
> 
> On 2020-02-27 5:22 PM, Roi Dayan wrote:
>> From: Dmytro Linkin <dmitrolin at mellanox.com>
>>
>> OVS can fail to attach ingress block on iface when init tc flow api,
>> if block already exist with rules on it and is shared with other iface.
>> Fix by flush all existing rules on the ingress block prior to deleting
>> it.
>>
>> Fixes: 093c9458fb02 ("tc: allow offloading of block ids")
>> Signed-off-by: Dmytro Linkin <dmitrolin at mellanox.com>
>> Acked-by: Raed Salem <raeds at mellanox.com>
>> Acked-by: Roi Dayan <roid at mellanox.com>
>> ---
>>  lib/netdev-offload-tc.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>> index 550e440b3a45..b5ff6ccca55e 100644
>> --- a/lib/netdev-offload-tc.c
>> +++ b/lib/netdev-offload-tc.c
>> @@ -1907,6 +1907,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>>      static struct ovsthread_once block_once = OVSTHREAD_ONCE_INITIALIZER;
>>      enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
>>      uint32_t block_id = 0;
>> +    struct tcf_id id;
>>      int ifindex;
>>      int error;
>>  
>> @@ -1917,6 +1918,14 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>>          return -ifindex;
>>      }
>>  
>> +    block_id = get_block_id_from_netdev(netdev);
>> +
>> +    /* Flush rules explicitly needed when we work with ingress_block,
>> +     * so we will not fail with reattaching block to bond iface, for ex.
>> +     */
>> +    id = tc_make_tcf_id(ifindex, block_id, 0, hook);
>> +    tc_del_filter(&id);
>> +
>>      /* make sure there is no ingress/egress qdisc */
>>      tc_add_del_qdisc(ifindex, false, 0, hook);
>>  
>> @@ -1930,7 +1939,6 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>>          ovsthread_once_done(&multi_mask_once);
>>      }
>>  
>> -    block_id = get_block_id_from_netdev(netdev);
>>      error = tc_add_del_qdisc(ifindex, true, block_id, hook);
>>  
>>      if (error && error != EEXIST) {
>>
> 
> +ilya
> 
> Hi Ilya,
> 
> can you help review/ack this?

Hi.  I'm not an expert in linux tc, but since you're asking...

IIUC, the issue is that tc_add_del_qdisc(ifindex, true, block_id, hook)
fails on bond iface.  Is it correct?

In this case I see one strange thing.  We're clearing ingress qdisk
for block_id == 0, but after that trying to create new one with
block_id == ifindex (for LAG interface).  Will it help if we delete
ingress qdisk providing correct block_id?  This sounds like something
sane to do.

What do you think?

Best regards, Ilya Maximets.


More information about the dev mailing list