| Message ID | 1429024817-21561-4-git-send-email-honli@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 14.04, Honggang Li wrote: > Signed-off-by: Honggang Li <honli@redhat.com> > --- > drivers/net/macvlan.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index b227a13..1e59f39 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -998,7 +998,9 @@ static int macvlan_dev_get_iflink(const struct net_device *dev) > { > struct macvlan_dev *vlan = netdev_priv(dev); > > - return vlan->lowerdev->ifindex; > + if (vlan && vlan->lowerdev) > + return vlan->lowerdev->ifindex; That is completely useless. vlan (=netdev_priv) can not be NULL as netdev_priv() never returns NULL and vlan->lowerdev is always valid because a macvlan wouldn't make much sense otherwise. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote: > > That is completely useless. vlan (=netdev_priv) can not be NULL as > netdev_priv() never returns NULL and vlan->lowerdev is always valid > because a macvlan wouldn't make much sense otherwise. OK, please drop this patch. thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 14/04/2015 17:26, Patrick McHardy a écrit : > On 14.04, Honggang Li wrote: [snip] >> - return vlan->lowerdev->ifindex; >> + if (vlan && vlan->lowerdev) >> + return vlan->lowerdev->ifindex; > > That is completely useless. vlan (=netdev_priv) can not be NULL as > netdev_priv() never returns NULL and vlan->lowerdev is always valid > because a macvlan wouldn't make much sense otherwise. > And I suspect that it is the same for ipvlan and dsa. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14.04, Honggang LI wrote: > On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote: > > > > That is completely useless. vlan (=netdev_priv) can not be NULL as > > netdev_priv() never returns NULL and vlan->lowerdev is always valid > > because a macvlan wouldn't make much sense otherwise. > > OK, please drop this patch. Well, the fact that netdev_priv never return NULL implies all your patches need to be redone. And I suggest you check whether this condition can actually happen. Just because it can in one driver says nothing at all about others. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 14, 2015 at 05:35:12PM +0200, Nicolas Dichtel wrote: > Le 14/04/2015 17:26, Patrick McHardy a écrit : > >On 14.04, Honggang Li wrote: > [snip] > >>- return vlan->lowerdev->ifindex; > >>+ if (vlan && vlan->lowerdev) > >>+ return vlan->lowerdev->ifindex; > > > >That is completely useless. vlan (=netdev_priv) can not be NULL as > >netdev_priv() never returns NULL and vlan->lowerdev is always valid > >because a macvlan wouldn't make much sense otherwise. > > > And I suspect that it is the same for ipvlan and dsa. I agree about DSA. I don't see any way this could happen. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 14, 2015 at 05:37:57PM +0200, Andrew Lunn wrote: > > > > > And I suspect that it is the same for ipvlan and dsa. > > I agree about DSA. I don't see any way this could happen. > > Andrew I only keep the ipoib patch and drop the rest patches. thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Honggang LI <honli@redhat.com> Date: Tue, 14 Apr 2015 23:32:39 +0800 > On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote: >> >> That is completely useless. vlan (=netdev_priv) can not be NULL as >> netdev_priv() never returns NULL and vlan->lowerdev is always valid >> because a macvlan wouldn't make much sense otherwise. > > OK, please drop this patch. That's not how this works. When a patch series needs any chnages, you must make a fresh, new complete submission of the entire patch series. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index b227a13..1e59f39 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -998,7 +998,9 @@ static int macvlan_dev_get_iflink(const struct net_device *dev) { struct macvlan_dev *vlan = netdev_priv(dev); - return vlan->lowerdev->ifindex; + if (vlan && vlan->lowerdev) + return vlan->lowerdev->ifindex; + return 0; } static const struct ethtool_ops macvlan_ethtool_ops = {
Signed-off-by: Honggang Li <honli@redhat.com> --- drivers/net/macvlan.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)