| Message ID | 1478226529-26766-1-git-send-email-fgao@ikuai8.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Nov 4, 2016 at 10:28 AM, <fgao@ikuai8.com> wrote: > From: Gao Feng <fgao@ikuai8.com> > > When there is no existing macvlan port in lowdev, one new macvlan port > would be created. But it doesn't be destoried when something failed later. > It casues some memleak. > > Now add one flag to indicate if new macvlan port is created. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > drivers/net/macvlan.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 3234fcd..d2d6f12 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -1278,6 +1278,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > struct net_device *lowerdev; > int err; > int macmode; > + bool create = false; > > if (!tb[IFLA_LINK]) > return -EINVAL; > @@ -1304,12 +1305,18 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > err = macvlan_port_create(lowerdev); > if (err < 0) > return err; > + create = true; > } > port = macvlan_port_get_rtnl(lowerdev); > > /* Only 1 macvlan device can be created in passthru mode */ > - if (port->passthru) > - return -EINVAL; > + if (port->passthru) { > + /* The macvlan port must be not created this time, > + * still goto destroy_macvlan_port for readability. > + */ > + err = -EINVAL; > + goto destroy_macvlan_port; > + } > > vlan->lowerdev = lowerdev; > vlan->dev = dev; > @@ -1325,24 +1332,28 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]); > > if (vlan->mode == MACVLAN_MODE_PASSTHRU) { > - if (port->count) > - return -EINVAL; > + if (port->count) { > + err = -EINVAL; > + goto destroy_macvlan_port; > + } > port->passthru = true; > eth_hw_addr_inherit(dev, lowerdev); > } > > if (data && data[IFLA_MACVLAN_MACADDR_MODE]) { > - if (vlan->mode != MACVLAN_MODE_SOURCE) > - return -EINVAL; > + if (vlan->mode != MACVLAN_MODE_SOURCE) { > + err = -EINVAL; > + goto destroy_macvlan_port; > + } > macmode = nla_get_u32(data[IFLA_MACVLAN_MACADDR_MODE]); > err = macvlan_changelink_sources(vlan, macmode, data); > if (err) > - return err; > + goto destroy_macvlan_port; > } > > err = register_netdevice(dev); > if (err < 0) > - return err; > + goto destroy_macvlan_port; > > dev->priv_flags |= IFF_MACVLAN; > err = netdev_upper_dev_link(lowerdev, dev); > @@ -1357,7 +1368,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, > > unregister_netdev: > unregister_netdevice(dev); > - > +destroy_macvlan_port: > + if (create) > + macvlan_port_destroy(port->dev); > return err; > } > EXPORT_SYMBOL_GPL(macvlan_common_newlink); > -- > 1.9.1 > > Hi all, How about this fix ? Is there any issue ? Regards Feng
From: fgao@ikuai8.com Date: Fri, 4 Nov 2016 10:28:49 +0800 > From: Gao Feng <fgao@ikuai8.com> > > When there is no existing macvlan port in lowdev, one new macvlan port > would be created. But it doesn't be destoried when something failed later. > It casues some memleak. > > Now add one flag to indicate if new macvlan port is created. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> You need to be more patient, it sometimes take several days before your get patch reviewed or applied. Sometimes nobody reviews a change for some time because it is obscure or everyone is busy. All patches are tracked in patchwork, so it is never an issue of a change getting "lost". Therefore, it never makes sense to ping the list again and ask if a change is "ok". Personally, when people ping like that, it makes me want to review that patch _less_ not more. So please do not do it. Thank you.
Hi David, On Tue, Nov 8, 2016 at 9:33 AM, David Miller <davem@davemloft.net> wrote: > From: fgao@ikuai8.com > Date: Fri, 4 Nov 2016 10:28:49 +0800 > >> From: Gao Feng <fgao@ikuai8.com> >> >> When there is no existing macvlan port in lowdev, one new macvlan port >> would be created. But it doesn't be destoried when something failed later. >> It casues some memleak. >> >> Now add one flag to indicate if new macvlan port is created. >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> > > You need to be more patient, it sometimes take several days before > your get patch reviewed or applied. Sometimes nobody reviews a change > for some time because it is obscure or everyone is busy. Sorry, it is my fault. I thought this patch may be lost because there are too many emails every day. So ping just as a reminder. > > All patches are tracked in patchwork, so it is never an issue of a > change getting "lost". Therefore, it never makes sense to ping the > list again and ask if a change is "ok". > > Personally, when people ping like that, it makes me want to review > that patch _less_ not more. So please do not do it. > > Thank you. > Now I get it. I violated some rules as a fresh man in kernel community. Sorry again. Best Regards Feng
From: fgao@ikuai8.com Date: Fri, 4 Nov 2016 10:28:49 +0800 > From: Gao Feng <fgao@ikuai8.com> > > When there is no existing macvlan port in lowdev, one new macvlan port > would be created. But it doesn't be destoried when something failed later. > It casues some memleak. > > Now add one flag to indicate if new macvlan port is created. > > Signed-off-by: Gao Feng <fgao@ikuai8.com> Looks good, applied, thanks.
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 3234fcd..d2d6f12 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -1278,6 +1278,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, struct net_device *lowerdev; int err; int macmode; + bool create = false; if (!tb[IFLA_LINK]) return -EINVAL; @@ -1304,12 +1305,18 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, err = macvlan_port_create(lowerdev); if (err < 0) return err; + create = true; } port = macvlan_port_get_rtnl(lowerdev); /* Only 1 macvlan device can be created in passthru mode */ - if (port->passthru) - return -EINVAL; + if (port->passthru) { + /* The macvlan port must be not created this time, + * still goto destroy_macvlan_port for readability. + */ + err = -EINVAL; + goto destroy_macvlan_port; + } vlan->lowerdev = lowerdev; vlan->dev = dev; @@ -1325,24 +1332,28 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]); if (vlan->mode == MACVLAN_MODE_PASSTHRU) { - if (port->count) - return -EINVAL; + if (port->count) { + err = -EINVAL; + goto destroy_macvlan_port; + } port->passthru = true; eth_hw_addr_inherit(dev, lowerdev); } if (data && data[IFLA_MACVLAN_MACADDR_MODE]) { - if (vlan->mode != MACVLAN_MODE_SOURCE) - return -EINVAL; + if (vlan->mode != MACVLAN_MODE_SOURCE) { + err = -EINVAL; + goto destroy_macvlan_port; + } macmode = nla_get_u32(data[IFLA_MACVLAN_MACADDR_MODE]); err = macvlan_changelink_sources(vlan, macmode, data); if (err) - return err; + goto destroy_macvlan_port; } err = register_netdevice(dev); if (err < 0) - return err; + goto destroy_macvlan_port; dev->priv_flags |= IFF_MACVLAN; err = netdev_upper_dev_link(lowerdev, dev); @@ -1357,7 +1368,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, unregister_netdev: unregister_netdevice(dev); - +destroy_macvlan_port: + if (create) + macvlan_port_destroy(port->dev); return err; } EXPORT_SYMBOL_GPL(macvlan_common_newlink);