* [PATCH] net/bonding: fix socket_id type @ 2022-10-12 11:45 Markus Theil 2022-10-12 12:23 ` Ferruh Yigit 0 siblings, 1 reply; 4+ messages in thread From: Markus Theil @ 2022-10-12 11:45 UTC (permalink / raw) To: dev; +Cc: Chas Williams, Min Hu, Thorben Roemer From: Thorben Roemer <thorben.roemer@secunet.com> DPDK uses int or u32 in most other places for socket IDs. Fix compilation warnings by also using int in the bonding code. Signed-off-by: Thorben Roemer <thorben.roemer@secunet.com> --- drivers/net/bonding/rte_eth_bond.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h index 874aa91a5f..3ce2b29052 100644 --- a/drivers/net/bonding/rte_eth_bond.h +++ b/drivers/net/bonding/rte_eth_bond.h @@ -99,7 +99,7 @@ extern "C" { * Port Id of created rte_eth_dev on success, negative value otherwise */ int -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id); +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id); /** * Free a bonded rte_eth_dev device diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c index b44dd219cb..3c6e236382 100644 --- a/drivers/net/bonding/rte_eth_bond_api.c +++ b/drivers/net/bonding/rte_eth_bond_api.c @@ -148,7 +148,7 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id) } int -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id) { struct bond_dev_private *internals; struct rte_eth_dev *bond_dev; -- 2.38.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/bonding: fix socket_id type 2022-10-12 11:45 [PATCH] net/bonding: fix socket_id type Markus Theil @ 2022-10-12 12:23 ` Ferruh Yigit 2022-10-12 14:20 ` Markus Theil 0 siblings, 1 reply; 4+ messages in thread From: Ferruh Yigit @ 2022-10-12 12:23 UTC (permalink / raw) To: Markus Theil, dev; +Cc: Chas Williams, Min Hu, Thorben Roemer On 10/12/2022 12:45 PM, Markus Theil wrote: > From: Thorben Roemer <thorben.roemer@secunet.com> > > DPDK uses int or u32 in most other places for > socket IDs. Fix compilation warnings by also > using int in the bonding code. > Hi Markus, 'rte_eth_bond_create()' is part of API, so changing it impacts the users. Since 'rte_socket_id()' returns 'int', it is reasonable to make 'socket_id' parameter type 'int', but I am not sure if it worth the trouble it may cause in user end. Maybe we can announce the change in this release and update the API in v23.11? Can you please list the mentioned compile warning? > Signed-off-by: Thorben Roemer <thorben.roemer@secunet.com> > --- > drivers/net/bonding/rte_eth_bond.h | 2 +- > drivers/net/bonding/rte_eth_bond_api.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond.h b/drivers/net/bonding/rte_eth_bond.h > index 874aa91a5f..3ce2b29052 100644 > --- a/drivers/net/bonding/rte_eth_bond.h > +++ b/drivers/net/bonding/rte_eth_bond.h > @@ -99,7 +99,7 @@ extern "C" { > * Port Id of created rte_eth_dev on success, negative value otherwise > */ > int > -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id); > +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id); > > /** > * Free a bonded rte_eth_dev device > diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c > index b44dd219cb..3c6e236382 100644 > --- a/drivers/net/bonding/rte_eth_bond_api.c > +++ b/drivers/net/bonding/rte_eth_bond_api.c > @@ -148,7 +148,7 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id) > } > > int > -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) > +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id) > { > struct bond_dev_private *internals; > struct rte_eth_dev *bond_dev; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/bonding: fix socket_id type 2022-10-12 12:23 ` Ferruh Yigit @ 2022-10-12 14:20 ` Markus Theil 2022-10-12 15:15 ` Ferruh Yigit 0 siblings, 1 reply; 4+ messages in thread From: Markus Theil @ 2022-10-12 14:20 UTC (permalink / raw) To: Ferruh Yigit, dev; +Cc: Chas Williams, Min Hu, Thorben Roemer On 10/12/22 14:23, Ferruh Yigit wrote: > On 10/12/2022 12:45 PM, Markus Theil wrote: >> From: Thorben Roemer <thorben.roemer@secunet.com> >> >> DPDK uses int or u32 in most other places for >> socket IDs. Fix compilation warnings by also >> using int in the bonding code. >> > > Hi Markus, > > 'rte_eth_bond_create()' is part of API, so changing it impacts the users. > > Since 'rte_socket_id()' returns 'int', it is reasonable to make > 'socket_id' parameter type 'int', but I am not sure if it worth the > trouble it may cause in user end. > > Maybe we can announce the change in this release and update the API in > v23.11? > Hi Ferruh, I've searched in the whole DPDK for the usage of socket IDs. Nearly every integral type pops up when doing this. We should postpone this patch. Maybe announce a cleanup + API change for the next release and fix this treewide (I can try to do this after 22.11 is out and you agree with the cleanup). > > Can you please list the mentioned compile warning? The warning only happens when we compile our code, which uses DPDK with Wconversion enabled. Our code stores socket ids as int. Therefore the compiler notices the int to u8 conversion and warns us. > >> Signed-off-by: Thorben Roemer <thorben.roemer@secunet.com> >> --- >> drivers/net/bonding/rte_eth_bond.h | 2 +- >> drivers/net/bonding/rte_eth_bond_api.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/bonding/rte_eth_bond.h >> b/drivers/net/bonding/rte_eth_bond.h >> index 874aa91a5f..3ce2b29052 100644 >> --- a/drivers/net/bonding/rte_eth_bond.h >> +++ b/drivers/net/bonding/rte_eth_bond.h >> @@ -99,7 +99,7 @@ extern "C" { >> * Port Id of created rte_eth_dev on success, negative value >> otherwise >> */ >> int >> -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id); >> +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id); >> /** >> * Free a bonded rte_eth_dev device >> diff --git a/drivers/net/bonding/rte_eth_bond_api.c >> b/drivers/net/bonding/rte_eth_bond_api.c >> index b44dd219cb..3c6e236382 100644 >> --- a/drivers/net/bonding/rte_eth_bond_api.c >> +++ b/drivers/net/bonding/rte_eth_bond_api.c >> @@ -148,7 +148,7 @@ deactivate_slave(struct rte_eth_dev *eth_dev, >> uint16_t port_id) >> } >> int >> -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) >> +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id) >> { >> struct bond_dev_private *internals; >> struct rte_eth_dev *bond_dev; > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/bonding: fix socket_id type 2022-10-12 14:20 ` Markus Theil @ 2022-10-12 15:15 ` Ferruh Yigit 0 siblings, 0 replies; 4+ messages in thread From: Ferruh Yigit @ 2022-10-12 15:15 UTC (permalink / raw) To: Markus Theil, dev, David Marchand Cc: Chas Williams, Min Hu, Thorben Roemer, Bruce Richardson, Thomas Monjalon On 10/12/2022 3:20 PM, Markus Theil wrote: > On 10/12/22 14:23, Ferruh Yigit wrote: >> On 10/12/2022 12:45 PM, Markus Theil wrote: >>> From: Thorben Roemer <thorben.roemer@secunet.com> >>> >>> DPDK uses int or u32 in most other places for >>> socket IDs. Fix compilation warnings by also >>> using int in the bonding code. >>> >> >> Hi Markus, >> >> 'rte_eth_bond_create()' is part of API, so changing it impacts the users. >> >> Since 'rte_socket_id()' returns 'int', it is reasonable to make >> 'socket_id' parameter type 'int', but I am not sure if it worth the >> trouble it may cause in user end. >> >> Maybe we can announce the change in this release and update the API in >> v23.11? >> > Hi Ferruh, > > I've searched in the whole DPDK for the usage of socket IDs. Nearly > every integral type pops up when doing this. We should postpone this > patch. Maybe announce a cleanup + API change for the next release and > fix this treewide (I can try to do this after 22.11 is out and you agree > with the cleanup). > I agree to cleanup. But unfortunately some of it is user interfacing and only can be done in ABI breaking releases, next one is 23.11. The process is: Send a patch to update deprecation notice ('doc/guides/rel_notes/deprecation.rst') explaining what will be done and why, with a target date. This patch needs to get three ack to be accepted. When it is accepted, this is the way to communicate with users about the oncoming change. In target release, for this case it is likely 23.11, do the batch cleanup and remove the deprecation note. Deprecation notice patch can trigger discussion and clarifies how to proceed. >> >> Can you please list the mentioned compile warning? > The warning only happens when we compile our code, which uses DPDK with > Wconversion enabled. Our code stores socket ids as int. Therefore the > compiler notices the int to u8 conversion and warns us. Got it, so there is nothing to fix in the upstream dpdk code. >> >>> Signed-off-by: Thorben Roemer <thorben.roemer@secunet.com> >>> --- >>> drivers/net/bonding/rte_eth_bond.h | 2 +- >>> drivers/net/bonding/rte_eth_bond_api.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/bonding/rte_eth_bond.h >>> b/drivers/net/bonding/rte_eth_bond.h >>> index 874aa91a5f..3ce2b29052 100644 >>> --- a/drivers/net/bonding/rte_eth_bond.h >>> +++ b/drivers/net/bonding/rte_eth_bond.h >>> @@ -99,7 +99,7 @@ extern "C" { >>> * Port Id of created rte_eth_dev on success, negative value >>> otherwise >>> */ >>> int >>> -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id); >>> +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id); >>> /** >>> * Free a bonded rte_eth_dev device >>> diff --git a/drivers/net/bonding/rte_eth_bond_api.c >>> b/drivers/net/bonding/rte_eth_bond_api.c >>> index b44dd219cb..3c6e236382 100644 >>> --- a/drivers/net/bonding/rte_eth_bond_api.c >>> +++ b/drivers/net/bonding/rte_eth_bond_api.c >>> @@ -148,7 +148,7 @@ deactivate_slave(struct rte_eth_dev *eth_dev, >>> uint16_t port_id) >>> } >>> int >>> -rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id) >>> +rte_eth_bond_create(const char *name, uint8_t mode, int socket_id) >>> { >>> struct bond_dev_private *internals; >>> struct rte_eth_dev *bond_dev; >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-12 15:15 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-12 11:45 [PATCH] net/bonding: fix socket_id type Markus Theil 2022-10-12 12:23 ` Ferruh Yigit 2022-10-12 14:20 ` Markus Theil 2022-10-12 15:15 ` Ferruh Yigit
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).