| Message ID | 1429191274-11600-1-git-send-email-erezsh@mellanox.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, Le jeudi 16 avril 2015 à 16:34 +0300, Erez Shitrit a écrit : > Currently, iflink of the parent interface was always accessed, even > when interface didn't have a parent and hence we crashed there. + since commit 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink'). as there was no crash here before AFAIK. > > Handle the interface types properly: for a child interface, return > the ifindex of the parent, for parent interface, return its ifindex. > > For child devices, make sure to set the parent pointer prior to > invoking register_netdevice(), this allows the new ndo to be called > by the stack immediately after the child device is registered. > > Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com> > Reported-by: Honggang Li <honli@redhat.com> > Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > Signed-off-by: Honggang Li <honli@redhat.com> > --- > > changes from V0: > - fixed two typos in the change-log > > drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 +++++ > drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 3 +-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c > index 657b89b..915ad04 100644 > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c > @@ -846,6 +846,11 @@ static int ipoib_get_iflink(const struct net_device *dev) > { > struct ipoib_dev_priv *priv = netdev_priv(dev); > > + /* parent interface */ > + if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) > + return dev->ifindex; > + > + /* child/vlan interface */ > return priv->parent->ifindex; > } > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c > index 4dd1313..fca1a88 100644 > --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c > +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c > @@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, > /* MTU will be reset when mcast join happens */ > priv->dev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu); > priv->mcast_mtu = priv->admin_mtu = priv->dev->mtu; > + priv->parent = ppriv->dev; > set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags); > > result = ipoib_set_dev_features(priv, ppriv->ca); > @@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, > goto register_failed; > } > > - priv->parent = ppriv->dev; > - > ipoib_create_debug_files(priv->dev); > > /* RTNL childs don't need proprietary sysfs entries */ Regards.
On Thu, Apr 16, 2015 at 04:34:34PM +0300, Erez Shitrit wrote: > Currently, iflink of the parent interface was always accessed, even > when interface didn't have a parent and hence we crashed there. > > Handle the interface types properly: for a child interface, return > the ifindex of the parent, for parent interface, return its ifindex. > > For child devices, make sure to set the parent pointer prior to > invoking register_netdevice(), this allows the new ndo to be called > by the stack immediately after the child device is registered. > > Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') > Reported-by: Honggang Li <honli@redhat.com> > Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > Signed-off-by: Honggang Li <honli@redhat.com> Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>+ > changes from V0: > - fixed two typos in the change-log > > drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 +++++ > drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 3 +-- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c > index 657b89b..915ad04 100644 > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c > @@ -846,6 +846,11 @@ static int ipoib_get_iflink(const struct net_device *dev) > { > struct ipoib_dev_priv *priv = netdev_priv(dev); > > + /* parent interface */ > + if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) > + return dev->ifindex; > + > + /* child/vlan interface */ > return priv->parent->ifindex; > } > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c > index 4dd1313..fca1a88 100644 > +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c > @@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, > /* MTU will be reset when mcast join happens */ > priv->dev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu); > priv->mcast_mtu = priv->admin_mtu = priv->dev->mtu; > + priv->parent = ppriv->dev; > set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags); > > result = ipoib_set_dev_features(priv, ppriv->ca); > @@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, > goto register_failed; > } > > - priv->parent = ppriv->dev; > - > ipoib_create_debug_files(priv->dev); > > /* RTNL childs don't need proprietary sysfs entries */ -- 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 Thu, Apr 16, 2015, Yann Droneaud <ydroneaud@opteya.com> wrote: > Hi, > > Le jeudi 16 avril 2015 à 16:34 +0300, Erez Shitrit a écrit : >> Currently, iflink of the parent interface was always accessed, even >> when interface didn't have a parent and hence we crashed there. > > + since commit 5aa7add8f14b ('infiniband/ipoib: implement > ndo_get_iflink'). > > as there was no crash here before AFAIK. Disagree, it's redundant, as the Fixes tag below points to this commit >> Handle the interface types properly: for a child interface, return >> the ifindex of the parent, for parent interface, return its ifindex. >> >> For child devices, make sure to set the parent pointer prior to >> invoking register_netdevice(), this allows the new ndo to be called >> by the stack immediately after the child device is registered. >> >> Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') > > Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com> Ditto, I find it enough to just copy Nicholas on the patch submission, as we did here. Or. >> Reported-by: Honggang Li <honli@redhat.com> >> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> >> Signed-off-by: Honggang Li <honli@redhat.com> >> --- >> >> changes from V0: >> - fixed two typos in the change-log >> >> drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 +++++ >> drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 3 +-- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c >> index 657b89b..915ad04 100644 >> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c >> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c >> @@ -846,6 +846,11 @@ static int ipoib_get_iflink(const struct net_device *dev) >> { >> struct ipoib_dev_priv *priv = netdev_priv(dev); >> >> + /* parent interface */ >> + if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) >> + return dev->ifindex; >> + >> + /* child/vlan interface */ >> return priv->parent->ifindex; >> } >> >> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c >> index 4dd1313..fca1a88 100644 >> --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c >> +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c >> @@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, >> /* MTU will be reset when mcast join happens */ >> priv->dev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu); >> priv->mcast_mtu = priv->admin_mtu = priv->dev->mtu; >> + priv->parent = ppriv->dev; >> set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags); >> >> result = ipoib_set_dev_features(priv, ppriv->ca); >> @@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, >> goto register_failed; >> } >> >> - priv->parent = ppriv->dev; >> - >> ipoib_create_debug_files(priv->dev); >> >> /* RTNL childs don't need proprietary sysfs entries */ > > Regards. > > -- > Yann Droneaud > OPTEYA > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 16/04/2015 15:34, Erez Shitrit a écrit : > Currently, iflink of the parent interface was always accessed, even > when interface didn't have a parent and hence we crashed there. > > Handle the interface types properly: for a child interface, return > the ifindex of the parent, for parent interface, return its ifindex. > > For child devices, make sure to set the parent pointer prior to > invoking register_netdevice(), this allows the new ndo to be called > by the stack immediately after the child device is registered. > > Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') > Reported-by: Honggang Li <honli@redhat.com> > Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > Signed-off-by: Honggang Li <honli@redhat.com> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> -- 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: Erez Shitrit <erezsh@mellanox.com> Date: Thu, 16 Apr 2015 16:34:34 +0300 > Currently, iflink of the parent interface was always accessed, even > when interface didn't have a parent and hence we crashed there. > > Handle the interface types properly: for a child interface, return > the ifindex of the parent, for parent interface, return its ifindex. > > For child devices, make sure to set the parent pointer prior to > invoking register_netdevice(), this allows the new ndo to be called > by the stack immediately after the child device is registered. > > Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') > Reported-by: Honggang Li <honli@redhat.com> > Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > Signed-off-by: Honggang Li <honli@redhat.com> Applied, 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
On 17/04/2015 22:21, David Miller wrote: > From: Erez Shitrit <erezsh@mellanox.com> > Date: Thu, 16 Apr 2015 16:34:34 +0300 > >> Currently, iflink of the parent interface was always accessed, even >> when interface didn't have a parent and hence we crashed there. >> >> Handle the interface types properly: for a child interface, return >> the ifindex of the parent, for parent interface, return its ifindex. >> >> For child devices, make sure to set the parent pointer prior to >> invoking register_netdevice(), this allows the new ndo to be called >> by the stack immediately after the child device is registered. >> >> Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') >> Reported-by: Honggang Li <honli@redhat.com> >> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> >> Signed-off-by: Honggang Li <honli@redhat.com> > > Applied, thanks. Doug, Roland, You might want to include this patch in your for-next / for-4.1 trees, or merge net-next again. Currently they contain the issue it fixes, and it can prevent some systems with IPoIB from booting. Regards, Haggai -- 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 Mon, Apr 20, 2015 at 11:16 AM, Haggai Eran <haggaie@mellanox.com> wrote: > On 17/04/2015 22:21, David Miller wrote: >> From: Erez Shitrit <erezsh@mellanox.com> >> Date: Thu, 16 Apr 2015 16:34:34 +0300 >> >>> Currently, iflink of the parent interface was always accessed, even >>> when interface didn't have a parent and hence we crashed there. >>> >>> Handle the interface types properly: for a child interface, return >>> the ifindex of the parent, for parent interface, return its ifindex. >>> >>> For child devices, make sure to set the parent pointer prior to >>> invoking register_netdevice(), this allows the new ndo to be called >>> by the stack immediately after the child device is registered. >>> >>> Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') >>> Reported-by: Honggang Li <honli@redhat.com> >>> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> >>> Signed-off-by: Honggang Li <honli@redhat.com> >> >> Applied, thanks. > > Doug, Roland, > You might want to include this patch in your for-next / for-4.1 trees, > or merge net-next again. Currently they contain the issue it fixes, and > it can prevent some systems with IPoIB from booting. Haggai, It's upstream by now, pull Linus tree. Or. -- 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 Mon, 2015-04-20 at 12:21 +0300, Or Gerlitz wrote: > On Mon, Apr 20, 2015 at 11:16 AM, Haggai Eran <haggaie@mellanox.com> wrote: > > On 17/04/2015 22:21, David Miller wrote: > >> From: Erez Shitrit <erezsh@mellanox.com> > >> Date: Thu, 16 Apr 2015 16:34:34 +0300 > >> > >>> Currently, iflink of the parent interface was always accessed, even > >>> when interface didn't have a parent and hence we crashed there. > >>> > >>> Handle the interface types properly: for a child interface, return > >>> the ifindex of the parent, for parent interface, return its ifindex. > >>> > >>> For child devices, make sure to set the parent pointer prior to > >>> invoking register_netdevice(), this allows the new ndo to be called > >>> by the stack immediately after the child device is registered. > >>> > >>> Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') > >>> Reported-by: Honggang Li <honli@redhat.com> > >>> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > >>> Signed-off-by: Honggang Li <honli@redhat.com> > >> > >> Applied, thanks. > > > > Doug, Roland, > > You might want to include this patch in your for-next / for-4.1 trees, > > or merge net-next again. Currently they contain the issue it fixes, and > > it can prevent some systems with IPoIB from booting. > > Haggai, > > > It's upstream by now, pull Linus tree. > > Or. Right, it already went via net-next. I skipped it because of that.
From: Haggai Eran <haggaie@mellanox.com> Date: Mon, 20 Apr 2015 11:16:34 +0300 > On 17/04/2015 22:21, David Miller wrote: >> From: Erez Shitrit <erezsh@mellanox.com> >> Date: Thu, 16 Apr 2015 16:34:34 +0300 >> >>> Currently, iflink of the parent interface was always accessed, even >>> when interface didn't have a parent and hence we crashed there. >>> >>> Handle the interface types properly: for a child interface, return >>> the ifindex of the parent, for parent interface, return its ifindex. >>> >>> For child devices, make sure to set the parent pointer prior to >>> invoking register_netdevice(), this allows the new ndo to be called >>> by the stack immediately after the child device is registered. >>> >>> Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') >>> Reported-by: Honggang Li <honli@redhat.com> >>> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> >>> Signed-off-by: Honggang Li <honli@redhat.com> >> >> Applied, thanks. > > Doug, Roland, > > You might want to include this patch in your for-next / for-4.1 trees, > or merge net-next again. Currently they contain the issue it fixes, and > it can prevent some systems with IPoIB from booting. I put this into 'net', not 'net-next'. 'net-next' is dormant after I do my first push to Linus of the merge window. After that everything goes via 'net' until the merge window closes and I open 'net-next' up again. -- 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/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 657b89b..915ad04 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -846,6 +846,11 @@ static int ipoib_get_iflink(const struct net_device *dev) { struct ipoib_dev_priv *priv = netdev_priv(dev); + /* parent interface */ + if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags)) + return dev->ifindex; + + /* child/vlan interface */ return priv->parent->ifindex; } diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c index 4dd1313..fca1a88 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c @@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, /* MTU will be reset when mcast join happens */ priv->dev->mtu = IPOIB_UD_MTU(priv->max_ib_mtu); priv->mcast_mtu = priv->admin_mtu = priv->dev->mtu; + priv->parent = ppriv->dev; set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags); result = ipoib_set_dev_features(priv, ppriv->ca); @@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv, goto register_failed; } - priv->parent = ppriv->dev; - ipoib_create_debug_files(priv->dev); /* RTNL childs don't need proprietary sysfs entries */
Currently, iflink of the parent interface was always accessed, even when interface didn't have a parent and hence we crashed there. Handle the interface types properly: for a child interface, return the ifindex of the parent, for parent interface, return its ifindex. For child devices, make sure to set the parent pointer prior to invoking register_netdevice(), this allows the new ndo to be called by the stack immediately after the child device is registered. Fixes: 5aa7add8f14b ('infiniband/ipoib: implement ndo_get_iflink') Reported-by: Honggang Li <honli@redhat.com> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> Signed-off-by: Honggang Li <honli@redhat.com> --- changes from V0: - fixed two typos in the change-log drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 +++++ drivers/infiniband/ulp/ipoib/ipoib_vlan.c | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-)