IMO we should figure out a name for the “may_steal” that carries the intended semantics a little bit better. Plain “steal” instead of “may_steal” would be better, but maybe not the best choice.
Acked-by: Jarno Rajahalme <ja...@ovn.org> > On Feb 8, 2016, at 10:43 PM, Ben Pfaff <b...@ovn.org> wrote: > > This manifested as a memory leak in test 898 "ofproto-dpif - sFlow packet > sampling - tunnel set", which included an output to a tunnel vport that > doesn't have an implementation of netdev_send(). > > Reported-by: William Tu <u9012...@gmail.com> > Reported-at: http://openvswitch.org/pipermail/dev/2016-February/065873.html > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > lib/netdev-provider.h | 6 ++++-- > lib/netdev.c | 20 ++++++++++++++------ > 2 files changed, 18 insertions(+), 8 deletions(-) > > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h > index d324ffc..2aa1b5d 100644 > --- a/lib/netdev-provider.h > +++ b/lib/netdev-provider.h > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -309,7 +309,9 @@ struct netdev_class { > * If the function returns a non-zero value, some of the packets might > have > * been sent anyway. > * > - * To retain ownership of 'buffers' caller can set may_steal to false. > + * If 'may_steal' is false, the caller retains ownership of all the > + * packets. If 'may_steal' is true, the caller transfers ownership of > all > + * the packets to the network device, regardless of success. > * > * The network device is expected to maintain one or more packet > * transmission queues, so that the caller does not ordinarily have to > diff --git a/lib/netdev.c b/lib/netdev.c > index c250c93..e27f854 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -728,7 +728,9 @@ netdev_set_multiq(struct netdev *netdev, unsigned int > n_txq, > * If the function returns a non-zero value, some of the packets might have > * been sent anyway. > * > - * To retain ownership of 'buffer' caller can set may_steal to false. > + * If 'may_steal' is false, the caller retains ownership of all the packets. > + * If 'may_steal' is true, the caller transfers ownership of all the packets > + * to the network device, regardless of success. > * > * The network device is expected to maintain one or more packet > * transmission queues, so that the caller does not ordinarily have to > @@ -742,11 +744,17 @@ int > netdev_send(struct netdev *netdev, int qid, struct dp_packet **buffers, > int cnt, bool may_steal) > { > - int error; > + if (!netdev->netdev_class->send) { > + if (may_steal) { > + for (int i = 0; i < cnt; i++) { > + dp_packet_delete(buffers[i]); > + } > + } > + return EOPNOTSUPP; > + } > > - error = (netdev->netdev_class->send > - ? netdev->netdev_class->send(netdev, qid, buffers, cnt, > may_steal) > - : EOPNOTSUPP); > + int error = netdev->netdev_class->send(netdev, qid, buffers, cnt, > + may_steal); > if (!error) { > COVERAGE_INC(netdev_sent); > } > -- > 2.1.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
