[ovs-dev] [PATCH V3 1/4] dpif-netdev: Flush offload rules upon port deletion

Finn, Emma emma.finn at intel.com
Wed Jan 13 08:52:15 UTC 2021



> -----Original Message-----
> From: dev <ovs-dev-bounces at openvswitch.org> On Behalf Of Eli Britstein
> Sent: Monday 4 January 2021 13:46
> To: Tonghao Zhang <xiangxia.m.yue at gmail.com>
> Cc: ovs dev <dev at openvswitch.org>; Gaetan Rivet <gaetanr at nvidia.com>; Ilya
> Maximets <i.maximets at ovn.org>
> Subject: Re: [ovs-dev] [PATCH V3 1/4] dpif-netdev: Flush offload rules upon port
> deletion
> 
> 
> On 1/4/2021 3:33 PM, Tonghao Zhang wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Tue, Dec 29, 2020 at 2:17 AM Eli Britstein <elibr at nvidia.com> wrote:
> >> When a port is deleted, flow deletion requests are posted, and the
> >> netdev is removed from offload netdevs map. Following flow deletion
> >> handling may be done after the netdev has already been removed from
> >> the offload netdevs map, so the HW rule is not removed and the data
> >> object is not freed (memory leak). Flush offload rules upon port
> >> deletion, and disable pending handling of offloads to fix it.
> >>
> >> Signed-off-by: Eli Britstein <elibr at nvidia.com>
> >> Reviewed-by: Gaetan Rivet <gaetanr at nvidia.com>
> >> ---
> >>   lib/dpif-netdev.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> >> 300861ca5..3f0639fca 100644
> >> --- a/lib/dpif-netdev.c
> >> +++ b/lib/dpif-netdev.c
> >> @@ -2281,6 +2281,8 @@ static void
> >>   do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port)
> >>       OVS_REQUIRES(dp->port_mutex)
> >>   {
> >> +    netdev_flow_flush(port->netdev);
> >> +    netdev_uninit_flow_api(port->netdev);
> > Hi Eli
> > Why not invoke the netdev_flow_flush in netdev_ports_remove ?
> > I posted a patch for tc offload, but not accepted yet.
> > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> work.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F20200611103635.5336
> 7
> > -1-
> xiangxia.m.yue%40gmail.com%2F&data=04%7C01%7Celibr%40nvidia.com
> >
> %7C53c2ee7dfde349ce938908d8b0b56f01%7C43083d15727340c1b7db39efd9c
> cc17a
> >
> %7C0%7C0%7C637453640601739470%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAw
> >
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdat
> a=Ay
> > QckaIcBfDeGHAJSe43BNAzpCu2q9U3xWbYGzV9hbY%3D&reserved=0
> 
> Hi Tonghao,
> 
> Thanks for your review.
> 
> In dpif-netdev, we should be under port_mutex lock. It is not the case in your
> patch. Also, netdev_uninit_flow_api is called here to make sure no more
> offload requests are handled for this port, even the pending ones in the queue.
> netdev_flow_flush won't do anything if called afterwards.
> 
> Thanks,
> 
> Eli
> 

LGTM.
Tested with Intel XL710 Devices.
Acked-by: Emma Finn <emma.finn at intel.com>
Tested-by: Emma Finn <emma.finn at intel.com>

> >
> >>       hmap_remove(&dp->ports, &port->node);
> >>       seq_change(dp->port_seq);
> >>
> >> --
> >> 2.28.0.546.g385c171
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai
> >> l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=04%7C01%7Ce
> >>
> libr%40nvidia.com%7C53c2ee7dfde349ce938908d8b0b56f01%7C43083d15727
> 340
> >>
> c1b7db39efd9ccc17a%7C0%7C0%7C637453640601739470%7CUnknown%7CTW
> FpbGZsb
> >>
> 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> 3D
> >>
> %7C1000&sdata=YxomBNTgR5RK8jGHfjQ83UlkjrpXJOJx1ZtT1r67PKg%3D&
> amp;
> >> reserved=0
> >
> >
> > --
> > Best regards, Tonghao
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list