[6/8] examples/ip_pipeline: remove setting of PCI ID and address

Message ID 20210910022402.26620-7-chenbo.xia@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series Removal of PCI bus ABIs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chenbo Xia Sept. 10, 2021, 2:24 a.m. UTC
  PCI ID and address in structure rte_kni_conf are never used and
will be removed in kni library. So remove the setting of them
first in the example.

Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
---
 examples/ip_pipeline/kni.c | 16 ----------------
 1 file changed, 16 deletions(-)
  

Comments

David Marchand Sept. 10, 2021, 7:18 a.m. UTC | #1
On Fri, Sep 10, 2021 at 4:38 AM Chenbo Xia <chenbo.xia@intel.com> wrote:
>
> PCI ID and address in structure rte_kni_conf are never used and
> will be removed in kni library. So remove the setting of them
> first in the example.
>
> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>

(forgot to Cc: Ferruh in previous comment)
Ditto patch 5.

I would tag with a Fixes: on ethtool removal.
Plus, this is the same kind of cleanup wrt kni, you can squash those
two patches together.


Too bad we do not have a deprecation notice.
lib/kni/rte_kni.h:      struct rte_pci_addr addr; /* depreciated */
lib/kni/rte_kni.h:      struct rte_pci_id id; /* depreciated */

Can we plan for this cleanup?
  
Chenbo Xia Sept. 10, 2021, 8:21 a.m. UTC | #2
Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, September 10, 2021 3:18 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev <dev@dpdk.org>; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 6/8] examples/ip_pipeline: remove setting of
> PCI ID and address
> 
> On Fri, Sep 10, 2021 at 4:38 AM Chenbo Xia <chenbo.xia@intel.com> wrote:
> >
> > PCI ID and address in structure rte_kni_conf are never used and
> > will be removed in kni library. So remove the setting of them
> > first in the example.
> >
> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> 
> (forgot to Cc: Ferruh in previous comment)
> Ditto patch 5.
> 
> I would tag with a Fixes: on ethtool removal.
> Plus, this is the same kind of cleanup wrt kni, you can squash those
> two patches together.

OK. Will do.

> 
> 
> Too bad we do not have a deprecation notice.
> lib/kni/rte_kni.h:      struct rte_pci_addr addr; /* depreciated */
> lib/kni/rte_kni.h:      struct rte_pci_id id; /* depreciated */
> 
> Can we plan for this cleanup?

My bad. I did't notice this when I send the deprecation notice for PCI bus
last release.

If we all agree, we can make this happen 22.11 (Or this release if tech-board
agrees).

Thanks,
Chenbo

> 
> 
> --
> David Marchand
  
Chenbo Xia Sept. 17, 2021, 3:09 a.m. UTC | #3
Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, September 10, 2021 3:18 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev <dev@dpdk.org>; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 6/8] examples/ip_pipeline: remove setting of
> PCI ID and address
> 
> On Fri, Sep 10, 2021 at 4:38 AM Chenbo Xia <chenbo.xia@intel.com> wrote:
> >
> > PCI ID and address in structure rte_kni_conf are never used and
> > will be removed in kni library. So remove the setting of them
> > first in the example.
> >
> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> 
> (forgot to Cc: Ferruh in previous comment)
> Ditto patch 5.
> 
> I would tag with a Fixes: on ethtool removal.
> Plus, this is the same kind of cleanup wrt kni, you can squash those
> two patches together.

A question about squash patch 5 and 6, then how should I title the combined
patch? Because there're both test change and example change. I'd like know
your suggestion.

Thanks,
Chenbo

> 
> 
> Too bad we do not have a deprecation notice.
> lib/kni/rte_kni.h:      struct rte_pci_addr addr; /* depreciated */
> lib/kni/rte_kni.h:      struct rte_pci_id id; /* depreciated */
> 
> Can we plan for this cleanup?
> 
> 
> --
> David Marchand
  
David Marchand Sept. 17, 2021, 11:55 a.m. UTC | #4
Hi Chenbo,

On Fri, Sep 17, 2021 at 5:09 AM Xia, Chenbo <chenbo.xia@intel.com> wrote:
> > > PCI ID and address in structure rte_kni_conf are never used and
> > > will be removed in kni library. So remove the setting of them
> > > first in the example.
> > >
> > > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> >
> > (forgot to Cc: Ferruh in previous comment)
> > Ditto patch 5.
> >
> > I would tag with a Fixes: on ethtool removal.
> > Plus, this is the same kind of cleanup wrt kni, you can squash those
> > two patches together.
>
> A question about squash patch 5 and 6, then how should I title the combined
> patch? Because there're both test change and example change. I'd like know
> your suggestion.
> >

This is a cleanup on unused fields wrt KNI.
"kni: remove unused PCI info from test and example" lgtm.

Ferruh, opinion?

And mark with:
Fixes: ea6b39b5b847 ("kni: remove ethtool support")
  
Ferruh Yigit Sept. 17, 2021, 3:37 p.m. UTC | #5
On 9/17/2021 12:55 PM, David Marchand wrote:
> Hi Chenbo,
> 
> On Fri, Sep 17, 2021 at 5:09 AM Xia, Chenbo <chenbo.xia@intel.com> wrote:
>>>> PCI ID and address in structure rte_kni_conf are never used and
>>>> will be removed in kni library. So remove the setting of them
>>>> first in the example.
>>>>
>>>> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
>>>
>>> (forgot to Cc: Ferruh in previous comment)
>>> Ditto patch 5.
>>>
>>> I would tag with a Fixes: on ethtool removal.
>>> Plus, this is the same kind of cleanup wrt kni, you can squash those
>>> two patches together.
>>
>> A question about squash patch 5 and 6, then how should I title the combined
>> patch? Because there're both test change and example change. I'd like know
>> your suggestion.
>>>
> 
> This is a cleanup on unused fields wrt KNI.
> "kni: remove unused PCI info from test and example" lgtm.
> 
> Ferruh, opinion?
> 

Make sense to merge them and above suggestion looks good. Thanks.

> And mark with:
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
>
  

Patch

diff --git a/examples/ip_pipeline/kni.c b/examples/ip_pipeline/kni.c
index a2d3331cb0..fccecc3dc6 100644
--- a/examples/ip_pipeline/kni.c
+++ b/examples/ip_pipeline/kni.c
@@ -6,7 +6,6 @@ 
 #include <string.h>
 
 #include <rte_ethdev.h>
-#include <rte_bus_pci.h>
 #include <rte_string_fns.h>
 
 #include "kni.h"
@@ -100,16 +99,12 @@  kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 struct kni *
 kni_create(const char *name, struct kni_params *params)
 {
-	struct rte_eth_dev_info dev_info;
 	struct rte_kni_conf kni_conf;
 	struct rte_kni_ops kni_ops;
 	struct kni *kni;
 	struct mempool *mempool;
 	struct link *link;
 	struct rte_kni *k;
-	const struct rte_pci_device *pci_dev;
-	const struct rte_bus *bus = NULL;
-	int ret;
 
 	/* Check input params */
 	if ((name == NULL) ||
@@ -124,23 +119,12 @@  kni_create(const char *name, struct kni_params *params)
 		return NULL;
 
 	/* Resource create */
-	ret = rte_eth_dev_info_get(link->port_id, &dev_info);
-	if (ret != 0)
-		return NULL;
-
 	memset(&kni_conf, 0, sizeof(kni_conf));
 	strlcpy(kni_conf.name, name, RTE_KNI_NAMESIZE);
 	kni_conf.force_bind = params->force_bind;
 	kni_conf.core_id = params->thread_id;
 	kni_conf.group_id = link->port_id;
 	kni_conf.mbuf_size = mempool->buffer_size;
-	if (dev_info.device)
-		bus = rte_bus_find_by_device(dev_info.device);
-	if (bus && !strcmp(bus->name, "pci")) {
-		pci_dev = RTE_DEV_TO_PCI(dev_info.device);
-		kni_conf.addr = pci_dev->addr;
-		kni_conf.id = pci_dev->id;
-	}
 
 	memset(&kni_ops, 0, sizeof(kni_ops));
 	kni_ops.port_id = link->port_id;