On 3/3/20 8:58 AM, Roi Dayan wrote: > > > On 2020-02-27 5:22 PM, Roi Dayan wrote: >> From: Dmytro Linkin <[email protected]> >> >> 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 <[email protected]> >> Acked-by: Raed Salem <[email protected]> >> Acked-by: Roi Dayan <[email protected]> >> --- >> 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
