Thanks for the review. I don't think there's much value in rejecting ports that are part of other datapaths, although I did consider it. I think that can only matter if the user runs dpctl commands directly, and I'm not too concerned about it.
On Thu, Apr 16, 2015 at 01:03:46PM +0100, Daniele Di Proietto wrote: > Acked-by: Daniele Di Proietto <[email protected]> > > I wonder if we should also reject ports which have been already added to > other userspace > datapaths. It is hardly a problem, because we end up using a single datapath > most of the > times, so I think this fix is enough for now. > > Going through all the datapaths here complicates the code a lot, because we > need to hold > dp_netdev_mutex (which must be acquired before dp->port_mutex). Perhaps we > could add > another static data structure in dpif-netdev.c to keep track of all the > netdevs used > in all the datapaths. I can do it, if you want > > Daniele > > > On 15 Apr 2015, at 19:19, Ben Pfaff <[email protected]> wrote: > > > > Otherwise it is at least very confusing. > > > > Found during testing. An upcoming commit adds a test. > > > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > lib/dpif-netdev.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 2cdb2cd..2ff2eac 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -1035,7 +1035,10 @@ do_add_port(struct dp_netdev *dp, const char > > *devname, const char *type, > > int error; > > int i; > > > > - /* XXX reject devices already in some dp_netdev. */ > > + /* Reject devices already in 'dp'. */ > > + if (!get_port_by_name(dp, devname, &port)) { > > + return EEXIST; > > + } > > > > /* Open and validate network device. */ > > open_type = dpif_netdev_port_open_type(dp->class, type); > > -- > > 2.1.3 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=D8B6d63GmBT6qWev0XfXlqlttLo0bsODqNQjD4qf4Ys&s=DMNs3ouJMLWLBLXUQZlxpIURKD3Q8PoFICFmt1FfDDU&e= > > > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
