On 01/03/2016 09:10, "Kavanagh, Mark B" <mark.b.kavan...@intel.com> wrote:
>Hi Daniele,
>
>One trivial comment below, but other than that, LGTM.
>
>Cheers,
>Mark
>
>>
>>This fixes multiple error path mistakes in do_add_port, none of which
>>has been a problem in practice so far. This change will make it easier
>>for a following commit to return in case of error.
>>
>>Also, this removes an unneeded special case for tunnel ports.
>>
>>Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>>---
>> lib/dpif-netdev.c | 50
>>+++++++++++++++++++++++++++-----------------------
>> 1 file changed, 27 insertions(+), 23 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index a7e224a..914579d 100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -1106,27 +1106,28 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char
>>*type,
>> struct netdev *netdev;
>> enum netdev_flags flags;
>> const char *open_type;
>>- int error;
>>- int i;
>>+ int error = 0;
>>+ int i, n_open_rxqs;
>>
>> /* Reject devices already in 'dp'. */
>> if (!get_port_by_name(dp, devname, &port)) {
>>- return EEXIST;
>>+ error = EEXIST;
>>+ goto out;
>> }
>>
>> /* Open and validate network device. */
>> open_type = dpif_netdev_port_open_type(dp->class, type);
>> error = netdev_open(devname, open_type, &netdev);
>> if (error) {
>>- return error;
>>+ goto out;
>> }
>> /* XXX reject non-Ethernet devices */
>>
>> netdev_get_flags(netdev, &flags);
>> if (flags & NETDEV_LOOPBACK) {
>> VLOG_ERR("%s: cannot add a loopback device", devname);
>>- netdev_close(netdev);
>>- return EINVAL;
>>+ error = EINVAL;
>>+ goto out_close;
>> }
>>
>> if (netdev_is_pmd(netdev)) {
>>@@ -1134,7 +1135,8 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char
>>*type,
>>
>> if (n_cores == OVS_CORE_UNSPEC) {
>> VLOG_ERR("%s, cannot get cpu core info", devname);
>>- return ENOENT;
>>+ error = ENOENT;
>>+ goto out_close;
>> }
>> /* There can only be ovs_numa_get_n_cores() pmd threads,
>> * so creates a txq for each, and one extra for the non
>>@@ -1143,7 +1145,7 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char
>>*type,
>> netdev_requested_n_rxq(netdev));
>> if (error && (error != EOPNOTSUPP)) {
>> VLOG_ERR("%s, cannot set multiq", devname);
>>- return errno;
>>+ goto out_close;
>> }
>> }
>> port = xzalloc(sizeof *port);
>>@@ -1152,30 +1154,20 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char
>>*type,
>> port->rxq = xmalloc(sizeof *port->rxq * netdev_n_rxq(netdev));
>> port->type = xstrdup(type);
>> port->latest_requested_n_rxq = netdev_requested_n_rxq(netdev);
>>+ n_open_rxqs = 0;
>
>You could just initialize n_open_rxqs in its declaration, but no biggie.
Good point, done
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
