From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id B11AF2B96 for ; Fri, 4 Mar 2016 18:20:16 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 04 Mar 2016 09:20:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,536,1449561600"; d="scan'208";a="663907488" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by FMSMGA003.fm.intel.com with ESMTP; 04 Mar 2016 09:20:15 -0800 Received: from irsmsx111.ger.corp.intel.com (10.108.20.4) by irsmsx110.ger.corp.intel.com (163.33.3.25) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 4 Mar 2016 17:20:14 +0000 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.13]) by irsmsx111.ger.corp.intel.com ([169.254.2.127]) with mapi id 14.03.0248.002; Fri, 4 Mar 2016 17:20:14 +0000 From: "Iremonger, Bernard" To: "Yigit, Ferruh" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices Thread-Index: AQHRdjk1WJIhtBkBNkKXTKTzoAl3t59JhoZw Date: Fri, 4 Mar 2016 17:20:13 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C219FBFB9B@IRSMSX108.ger.corp.intel.com> References: <1456229580-25009-1-git-send-email-bernard.iremonger@intel.com> <56D9C23B.8000508@intel.com> In-Reply-To: <56D9C23B.8000508@intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmYwZDY0ZmItZTE1MC00NGJkLThlMDUtOWZlMDAzMDVkODlhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlJ1dXdQd2s1VWNTbzAyUnR2OXpcL2lmbkFYdVwvanh0c25JeXp6Rjd3TWtCZz0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Mar 2016 17:20:17 -0000 Hi Ferruh, > -----Original Message----- > From: Yigit, Ferruh > Sent: Friday, March 4, 2016 5:14 PM > To: Iremonger, Bernard ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] bonding: fix crash when no slave devices >=20 > 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 > > --- > > 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, >=20 > 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. =20 > 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 othe= r > cases. >=20 > Thanks, > Ferruh I will try initializing current_primary_port to -1 and see if this helps. Thanks for reviewing. Regards, Bernard.