On Fri, May 15, 2020 at 09:49:15PM +0200, Ilya Maximets wrote:
> On 5/7/20 5:54 PM, Aaron Conole wrote:
> > It's possible that port ordering could cause the block ID to change
> > after enabling / detecting TC Flower support. Therefore, fetch the
> > block_id again after probing.
> >
> > Fixes: edc2055a2bf7 ("netdev-offload-tc: Flush rules on ingress block when
> > init tc flow api")
> > Cc: Dmytro Linkin <dmitro...@mellanox.com>
> > Co-authored-by: Marcelo Leitner <mleit...@redhat.com>
> > Signed-off-by: Marcelo Leitner <mleit...@redhat.com>
> > Signed-off-by: Aaron Conole <acon...@redhat.com>
> > ---
> > lib/netdev-offload-tc.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 875ebef719..8864555d01 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1939,6 +1939,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> > ovsthread_once_done(&multi_mask_once);
> > }
> >
> > + block_id = get_block_id_from_netdev(netdev);
>
> This is not obvious from the code why we're fetching block id twice.
> Maybe we could move this right after probing and add the comment?
> Something like:
>
> if (ovsthread_once_start(&block_once)) {
> probe_tc_block_support(ifindex);
> /* Need to re-fetch block id since it depends on feature
> availability. */
> block_id = get_block_id_from_netdev(netdev);
> ovsthread_once_done(&block_once);
> }
>
> What do you think?
Yes, LGTM. Good point, it doesn't need to be re-fetched if the first
one was good already (i.e., if the block support was already checked).
Thanks,
Marcelo
>
> Best regards, Ilya Maximets.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
