* [dpdk-dev] [PATCH] bonding: fix crash when no slave devices
@ 2016-02-23 12:13 Bernard Iremonger
2016-03-04 17:13 ` Ferruh Yigit
2016-03-07 11:40 ` [dpdk-dev] [PATCH v2] " Bernard Iremonger
0 siblings, 2 replies; 6+ messages in thread
From: Bernard Iremonger @ 2016-02-23 12:13 UTC (permalink / raw)
To: dev
If a bonded device is created when there are no slave devices
there is loop in bond_ethdev_promiscous_enable() which results
in a segmentation fault.
I have applied a similar fix to bond_ethdev_promiscous_disable()
where a similar loop could occur.
Fixes: 2efb58cbab6e ("bond: new link bonding library")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b63c886..78972fc 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1870,7 +1870,8 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
case BONDING_MODE_TLB:
case BONDING_MODE_ALB:
default:
- rte_eth_promiscuous_enable(internals->current_primary_port);
+ if (internals->slave_count > 0)
+ rte_eth_promiscuous_enable(internals->current_primary_port);
}
}
@@ -1898,7 +1899,8 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev)
case BONDING_MODE_TLB:
case BONDING_MODE_ALB:
default:
- rte_eth_promiscuous_disable(internals->current_primary_port);
+ if (internals->slave_count > 0)
+ rte_eth_promiscuous_disable(internals->current_primary_port);
}
}
--
2.6.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices 2016-02-23 12:13 [dpdk-dev] [PATCH] bonding: fix crash when no slave devices Bernard Iremonger @ 2016-03-04 17:13 ` Ferruh Yigit 2016-03-04 17:20 ` Iremonger, Bernard 2016-03-07 11:40 ` [dpdk-dev] [PATCH v2] " Bernard Iremonger 1 sibling, 1 reply; 6+ messages in thread From: Ferruh Yigit @ 2016-03-04 17:13 UTC (permalink / raw) To: Bernard Iremonger, dev On 2/23/2016 12:13 PM, Bernard Iremonger wrote: > If a bonded device is created when there are no slave devices > there is loop in bond_ethdev_promiscous_enable() which results > in a segmentation fault. > I have applied a similar fix to bond_ethdev_promiscous_disable() > where a similar loop could occur. > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index b63c886..78972fc 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1870,7 +1870,8 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev) > case BONDING_MODE_TLB: > case BONDING_MODE_ALB: > default: > - rte_eth_promiscuous_enable(internals->current_primary_port); > + if (internals->slave_count > 0) > + rte_eth_promiscuous_enable(internals->current_primary_port); > } > } > > @@ -1898,7 +1899,8 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev) > case BONDING_MODE_TLB: > case BONDING_MODE_ALB: > default: > - rte_eth_promiscuous_disable(internals->current_primary_port); > + if (internals->slave_count > 0) > + rte_eth_promiscuous_disable(internals->current_primary_port); > } > } > > Hi Bernard, The reason of this crash is when there is no slave, the value of current_primary_port is 0, which is valid port_id, is this correct? Does it make sense, instead of slave_count check, to make default current_primary_port value a non valid port_id, like -1, so is_valid_port() check catches it to prevents crash? For this and any other cases. Thanks, ferruh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices 2016-03-04 17:13 ` Ferruh Yigit @ 2016-03-04 17:20 ` Iremonger, Bernard 0 siblings, 0 replies; 6+ messages in thread From: Iremonger, Bernard @ 2016-03-04 17:20 UTC (permalink / raw) To: Yigit, Ferruh, dev Hi Ferruh, > -----Original Message----- > From: Yigit, Ferruh > Sent: Friday, March 4, 2016 5:14 PM > To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices > > On 2/23/2016 12:13 PM, Bernard Iremonger wrote: > > If a bonded device is created when there are no slave devices there is > > loop in bond_ethdev_promiscous_enable() which results in a > > segmentation fault. > > I have applied a similar fix to bond_ethdev_promiscous_disable() where > > a similar loop could occur. > > > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > > --- > > drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > > b/drivers/net/bonding/rte_eth_bond_pmd.c > > index b63c886..78972fc 100644 > > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > @@ -1870,7 +1870,8 @@ bond_ethdev_promiscuous_enable(struct > rte_eth_dev *eth_dev) > > case BONDING_MODE_TLB: > > case BONDING_MODE_ALB: > > default: > > - rte_eth_promiscuous_enable(internals- > >current_primary_port); > > + if (internals->slave_count > 0) > > + rte_eth_promiscuous_enable(internals- > >current_primary_port); > > } > > } > > > > @@ -1898,7 +1899,8 @@ bond_ethdev_promiscuous_disable(struct > rte_eth_dev *dev) > > case BONDING_MODE_TLB: > > case BONDING_MODE_ALB: > > default: > > - rte_eth_promiscuous_disable(internals- > >current_primary_port); > > + if (internals->slave_count > 0) > > + rte_eth_promiscuous_disable(internals- > >current_primary_port); > > } > > } > > > > > Hi Bernard, > > The reason of this crash is when there is no slave, the value of > current_primary_port is 0, which is valid port_id, is this correct? Yes. > Does it make sense, instead of slave_count check, to make default > current_primary_port value a non valid port_id, like -1, so > is_valid_port() check catches it to prevents crash? For this and any other > cases. > > Thanks, > Ferruh I will try initializing current_primary_port to -1 and see if this helps. Thanks for reviewing. Regards, Bernard. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v2] bonding: fix crash when no slave devices 2016-02-23 12:13 [dpdk-dev] [PATCH] bonding: fix crash when no slave devices Bernard Iremonger 2016-03-04 17:13 ` Ferruh Yigit @ 2016-03-07 11:40 ` Bernard Iremonger 2016-03-07 12:14 ` Ferruh Yigit 1 sibling, 1 reply; 6+ messages in thread From: Bernard Iremonger @ 2016-03-07 11:40 UTC (permalink / raw) To: dev If a bonded device is created when there are no slave devices there is loop in bond_ethdev_promiscous_enable() which results in a segmentation fault. The solution is to initialise the current_primary_port to an invalid port value when the bonded port is created. Fixes: 2efb58cbab6e ("bond: new link bonding library") Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> --- Changes in V2: Set current_primary_port to an invalid value. drivers/net/bonding/rte_eth_bond_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index a0995ec..1d67a0c 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -231,7 +231,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) internals->port_id = eth_dev->data->port_id; internals->mode = BONDING_MODE_INVALID; - internals->current_primary_port = 0; + internals->current_primary_port = RTE_MAX_ETHPORTS + 1; internals->balance_xmit_policy = BALANCE_XMIT_POLICY_LAYER2; internals->xmit_hash = xmit_l2_hash; internals->user_defined_mac = 0; -- 2.6.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] bonding: fix crash when no slave devices 2016-03-07 11:40 ` [dpdk-dev] [PATCH v2] " Bernard Iremonger @ 2016-03-07 12:14 ` Ferruh Yigit 2016-03-11 16:40 ` Bruce Richardson 0 siblings, 1 reply; 6+ messages in thread From: Ferruh Yigit @ 2016-03-07 12:14 UTC (permalink / raw) To: Bernard Iremonger, dev On 3/7/2016 11:40 AM, Bernard Iremonger wrote: > If a bonded device is created when there are no slave devices > there is loop in bond_ethdev_promiscous_enable() which results > in a segmentation fault. > > The solution is to initialise the current_primary_port to an > invalid port value when the bonded port is created. > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] bonding: fix crash when no slave devices 2016-03-07 12:14 ` Ferruh Yigit @ 2016-03-11 16:40 ` Bruce Richardson 0 siblings, 0 replies; 6+ messages in thread From: Bruce Richardson @ 2016-03-11 16:40 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev On Mon, Mar 07, 2016 at 12:14:51PM +0000, Ferruh Yigit wrote: > On 3/7/2016 11:40 AM, Bernard Iremonger wrote: > > If a bonded device is created when there are no slave devices > > there is loop in bond_ethdev_promiscous_enable() which results > > in a segmentation fault. > > > > The solution is to initialise the current_primary_port to an > > invalid port value when the bonded port is created. > > > > Fixes: 2efb58cbab6e ("bond: new link bonding library") > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com> > > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> Applied to dpdk-next-net/rel_16_04 /Bruce ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-11 16:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-23 12:13 [dpdk-dev] [PATCH] bonding: fix crash when no slave devices Bernard Iremonger 2016-03-04 17:13 ` Ferruh Yigit 2016-03-04 17:20 ` Iremonger, Bernard 2016-03-07 11:40 ` [dpdk-dev] [PATCH v2] " Bernard Iremonger 2016-03-07 12:14 ` Ferruh Yigit 2016-03-11 16:40 ` Bruce Richardson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).