Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Yuanhan Liu]

On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
> 2016-08-31 14:06, Ferruh Yigit:
> > Removing KNI interface that has no PCI driver for ethtool support cause
> > kernel crash.
> > 
> > Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
> > Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
> > 
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Applied, thanks

FYI, this patch has been applied to stable branch v16.07.

It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
It will be pushed if I get no objections in around TWO days. So 
please shutout if you have objections.

Thanks.

	--yliu

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Yuanhan Liu]

On Mon, Sep 26, 2016 at 04:34:07PM +0800, Yuanhan Liu wrote:
> On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
> > 2016-08-31 14:06, Ferruh Yigit:
> > > Removing KNI interface that has no PCI driver for ethtool support cause
> > > kernel crash.
> > > 
> > > Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
> > > Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
> > > 
> > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > 
> > Applied, thanks
> 
> FYI, this patch has been applied to stable branch v16.07.
> 
> It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
> It will be pushed if I get no objections in around TWO days. So 
> please shutout if you have objections.

Nah, firstly, it has conflicts. Secondly, it breaks the build even
if I fixed the conflicts.

    lib/librte_eal/linuxapp/kni/kni_misc.c: In function ‘kni_dev_remove’:
    lib/librte_eal/linuxapp/kni/kni_misc.c:370:20: error: ‘ixgbe_pci_tbl’ undeclared (first use in this function)
       if (pci_match_id(ixgbe_pci_tbl, dev->pci_dev))
                        ^
    lib/librte_eal/linuxapp/kni/kni_misc.c:370:20: note: each undeclared identifier is reported only once for each function it appears in
    lib/librte_eal/linuxapp/kni/kni_misc.c:372:25: error: ‘igb_pci_tbl’ undeclared (first use in this function)
       else if (pci_match_id(igb_pci_tbl, dev->pci_dev))
                             ^
    make[10]: *** [lib/librte_eal/linuxapp/kni/kni_misc.o] Error 1
    make[9]: *** [_module_lib/librte_eal/linuxapp/kni] Error 2
    make[8]: *** [sub-make] Error 2
    make[7]: *** [rte_kni.ko] Error 2
    make[6]: *** [kni] Error 2
    make[5]: *** [linuxapp] Error 2
    make[4]: *** [librte_eal] Error 2
    make[3]: *** [lib] Error 2
    make[2]: *** [all] Error 2
    make[1]: *** [pre_install] Error 2
    make: *** [install] Error 2
    

Ferruh, I may need your help here. If you think this patch is a good
candidate for v16.07 stable release, would you mind to back port it?

BTW, I saw quite few KNI fixes have already been applied to master. Would
you help to check which are good for applying them to stable release and
which are not?

    2016-09-22 15:48 Ferruh Yigit       kni: fix large stack frame size
    2016-08-25 12:30 Ferruh Yigit       kni: remove continuous memory restriction
    2016-09-11 17:59 Yangchao Zhou      kni: fix error rollback kernel crash
    2016-09-08 19:32 Ferruh Yigit       kni: fix build with kernel 4.8
    2016-09-16 17:09 Ferruh Yigit       kni: fix build with kernel < 3.0
    2016-09-16 22:01 Pablo de Lara      kni: support RHEL 7.3
    2016-09-15 13:06 Ferruh Yigit       kni: fix debug build
    2016-09-02 18:23 Vincent Guo        kni: fix module init/exit
    2016-08-31 15:49 Ferruh Yigit       kni: support RHEL 6.8
    2016-08-31 14:06 Ferruh Yigit       kni: fix crash when removing interface

So far, I will drop this one temporarily.

	--yliu

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Ferruh Yigit]

On 9/26/2016 9:34 AM, Yuanhan Liu wrote:
> On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
>> 2016-08-31 14:06, Ferruh Yigit:
>>> Removing KNI interface that has no PCI driver for ethtool support cause
>>> kernel crash.
>>>
>>> Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
>>> Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Applied, thanks
> 
> FYI, this patch has been applied to stable branch v16.07.
> 
> It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
> It will be pushed if I get no objections in around TWO days. So 
> please shutout if you have objections.

If the patches mentioned in Fixes line applied to stable branch, this
one also should be applied.

Thanks,
ferruh

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Yuanhan Liu]

On Mon, Sep 26, 2016 at 10:21:12AM +0100, Ferruh Yigit wrote:
> On 9/26/2016 9:34 AM, Yuanhan Liu wrote:
> > On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
> >> 2016-08-31 14:06, Ferruh Yigit:
> >>> Removing KNI interface that has no PCI driver for ethtool support cause
> >>> kernel crash.
> >>>
> >>> Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
> >>> Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
> >>>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>
> >> Applied, thanks
> > 
> > FYI, this patch has been applied to stable branch v16.07.
> > 
> > It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
> > It will be pushed if I get no objections in around TWO days. So 
> > please shutout if you have objections.
> 
> If the patches mentioned in Fixes line applied to stable branch, this
> one also should be applied.

Thanks for the response.

It might address the conflict issue, but with a simple glimpse, the
build issue might still remain. Say, commit 109febfe58f9 ("net/igb: move
PCI device IDs from EAL") will not be applied (as there is no fixline),
but it is required by commit 5544a453b4ac ("kni: fix crash when removing
interface"), if I'm not mistaken.

Or, I'm wrong?

	--yliu

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Ferruh Yigit]

On 9/26/2016 1:15 PM, Yuanhan Liu wrote:
> On Mon, Sep 26, 2016 at 10:21:12AM +0100, Ferruh Yigit wrote:
>> On 9/26/2016 9:34 AM, Yuanhan Liu wrote:
>>> On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
>>>> 2016-08-31 14:06, Ferruh Yigit:
>>>>> Removing KNI interface that has no PCI driver for ethtool support cause
>>>>> kernel crash.
>>>>>
>>>>> Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
>>>>> Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
>>>>>
>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>
>>>> Applied, thanks
>>>
>>> FYI, this patch has been applied to stable branch v16.07.
>>>
>>> It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
>>> It will be pushed if I get no objections in around TWO days. So 
>>> please shutout if you have objections.
>>
>> If the patches mentioned in Fixes line applied to stable branch, this
>> one also should be applied.
> 
> Thanks for the response.
> 
> It might address the conflict issue, 

Which conflict issue?

> but with a simple glimpse, the
> build issue might still remain. Say, commit 109febfe58f9 ("net/igb: move
> PCI device IDs from EAL") will not be applied (as there is no fixline),

109febfe58f9 & 221fba3b987c are in fixes line of this patch.

> but it is required by commit 5544a453b4ac ("kni: fix crash when removing
> interface"), if I'm not mistaken.

I am not clear with question but, the issue is:
109febfe58f9 & 221fba3b987c creates a defect and this patch fixes it.

If both 109febfe58f9 or 221fba3b987c get into stable tree, this patch
needs to get in to fix crash. If you prefer to get one of them (although
that doesn't make much sense), a backported version of this patch needs
to be applied.

> 
> Or, I'm wrong?
> 
> 	--yliu
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Yuanhan Liu]

On Mon, Sep 26, 2016 at 01:26:20PM +0100, Ferruh Yigit wrote:
> On 9/26/2016 1:15 PM, Yuanhan Liu wrote:
> > On Mon, Sep 26, 2016 at 10:21:12AM +0100, Ferruh Yigit wrote:
> >> On 9/26/2016 9:34 AM, Yuanhan Liu wrote:
> >>> On Fri, Sep 09, 2016 at 03:46:01PM +0200, Thomas Monjalon wrote:
> >>>> 2016-08-31 14:06, Ferruh Yigit:
> >>>>> Removing KNI interface that has no PCI driver for ethtool support cause
> >>>>> kernel crash.
> >>>>>
> >>>>> Fixes: 109febfe58f9 ("net/igb: move PCI device IDs from EAL")
> >>>>> Fixes: 221fba3b987c ("net/ixgbe: move PCI device IDs from EAL")
> >>>>>
> >>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>
> >>>> Applied, thanks
> >>>
> >>> FYI, this patch has been applied to stable branch v16.07.
> >>>
> >>> It hasn't been pushed to http://dpdk.org/browse/dpdk-stable/ yet.
> >>> It will be pushed if I get no objections in around TWO days. So 
> >>> please shutout if you have objections.
> >>
> >> If the patches mentioned in Fixes line applied to stable branch, this
> >> one also should be applied.
> > 
> > Thanks for the response.
> > 
> > It might address the conflict issue, 
> 
> Which conflict issue?
> 
> > but with a simple glimpse, the
> > build issue might still remain. Say, commit 109febfe58f9 ("net/igb: move
> > PCI device IDs from EAL") will not be applied (as there is no fixline),
> 
> 109febfe58f9 & 221fba3b987c are in fixes line of this patch.

Oh, I see. You were fixing an issue that introduced in v16.11, but not
v16.07. Then there is no reason to put it into v16.07 stable release.
My bad, I should have checked the fixline more carefully!

BTW, may I ask some helps from you? As I said in another email, I saw
quite many bug fixing patches for KNI. Would you help to list few (if
any) that need be picked for v16.07 release? I mean, some bug fixing
patches that fix some bugs before v16.07 (inclusive).

	--yliu

> 
> > but it is required by commit 5544a453b4ac ("kni: fix crash when removing
> > interface"), if I'm not mistaken.
> 
> I am not clear with question but, the issue is:
> 109febfe58f9 & 221fba3b987c creates a defect and this patch fixes it.
> 
> If both 109febfe58f9 or 221fba3b987c get into stable tree, this patch
> needs to get in to fix crash. If you prefer to get one of them (although
> that doesn't make much sense), a backported version of this patch needs
> to be applied.
> 
> > 
> > Or, I'm wrong?
> > 
> > 	--yliu
> > 

Re: [dpdk-stable] [dpdk-dev] [PATCH] kni: fix crash for KNI interface remove

[Ferruh Yigit]

<...>

>>> but with a simple glimpse, the
>>> build issue might still remain. Say, commit 109febfe58f9 ("net/igb: move
>>> PCI device IDs from EAL") will not be applied (as there is no fixline),
>>
>> 109febfe58f9 & 221fba3b987c are in fixes line of this patch.
> 
> Oh, I see. You were fixing an issue that introduced in v16.11, but not
> v16.07.

Right.

<..>
> 
> BTW, may I ask some helps from you? As I said in another email, I saw
> quite many bug fixing patches for KNI. Would you help to list few (if
> any) that need be picked for v16.07 release? I mean, some bug fixing
> patches that fix some bugs before v16.07 (inclusive).

Sure, I will share the list.

<...>