| Message ID | 1611040501-11666-5-git-send-email-xuemingl@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Series | [v5,1/9] ethdev: introduce representor type | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
On 1/19/21 10:14 AM, Xueming Li wrote: > With Kernel bonding, multiple underlying PFs are bonded, VFs come > from different PF, need to identify representor of VFs unambiguously by > adding PF index. > > This patch introduces optional 'pf' section to representor devargs > syntax, examples: > representor=pf0vf0 - single VF representor > representor=pf[0-1]sf[0-1023] - SF representors from 2 PFs Don't we need representor=pf3 i.e. without VF or sub-function? > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> > Acked-by: Thomas Monjalon <thomas@monjalon.net> > --- > doc/guides/prog_guide/poll_mode_drv.rst | 2 ++ > lib/librte_ethdev/ethdev_private.c | 13 +++++++++++-- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst > index 86e5867f1b..b2147aad30 100644 > --- a/doc/guides/prog_guide/poll_mode_drv.rst > +++ b/doc/guides/prog_guide/poll_mode_drv.rst > @@ -382,6 +382,8 @@ parameters to those ports. > -a DBDF,representor=sf[1,3,5] > -a DBDF,representor=sf[0-1023] > -a DBDF,representor=sf[0,2-4,7,9-11] > + -a DBDF,representor=pf1vf0 > + -a DBDF,representor=pf[0-1]sf[0-127] > > Note: PMDs are not required to support the standard device arguments and users > should consult the relevant PMD documentation to see support devargs. > diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c > index d513f035d0..b9fdbd0f72 100644 > --- a/lib/librte_ethdev/ethdev_private.c > +++ b/lib/librte_ethdev/ethdev_private.c > @@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list, > * > * Representor format: > * #: range or single number of VF representor - legacy > - * vf#: VF port representor/s > - * sf#: SF port representor/s > + * [pf#]vf#: VF port representor/s > + * [pf#]sf#: SF port representor/s > * > * Examples of #: > * 2 - single > @@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data) > { > struct rte_eth_devargs *eth_da = data; > > + if (str[0] == 'p' && str[1] == 'f') { > + eth_da->type = RTE_ETH_REPRESENTOR_PF; > + str += 2; > + str = rte_eth_devargs_process_list(str, eth_da->ports, > + ð_da->nb_ports, RTE_MAX_ETHPORTS); May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ? > + if (str == NULL) > + goto err; Below we should not allow legacy VF syntax without "vf" prefix. > + } > if (str[0] == 'v' && str[1] == 'f') { > eth_da->type = RTE_ETH_REPRESENTOR_VF; > str += 2; > @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data) > } > str = rte_eth_devargs_process_list(str, eth_da->representor_ports, > ð_da->nb_representor_ports, RTE_MAX_ETHPORTS); > +err: > if (str == NULL) > RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); > return str == NULL ? -1 : 0; >
Hi Andrew, >-----Original Message----- >From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >Sent: Tuesday, January 19, 2021 4:01 PM >To: Xueming(Steven) Li <xuemingl@nvidia.com> >Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf Penso ><asafp@nvidia.com>; NBU-Contact-Thomas Monjalon ><thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com> >Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor > >On 1/19/21 10:14 AM, Xueming Li wrote: >> With Kernel bonding, multiple underlying PFs are bonded, VFs come from >> different PF, need to identify representor of VFs unambiguously by >> adding PF index. >> >> This patch introduces optional 'pf' section to representor devargs >> syntax, examples: >> representor=pf0vf0 - single VF representor >> representor=pf[0-1]sf[0-1023] - SF representors from 2 PFs > > >Don't we need > representor=pf3 >i.e. without VF or sub-function? Standalone PF not used by Mellnaox PMD, but should be supported. Will update. > >> >> >> Signed-off-by: Xueming Li <xuemingl@nvidia.com> >> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> >> Acked-by: Thomas Monjalon <thomas@monjalon.net> >> --- >> doc/guides/prog_guide/poll_mode_drv.rst | 2 ++ >> lib/librte_ethdev/ethdev_private.c | 13 +++++++++++-- >> 2 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst >> b/doc/guides/prog_guide/poll_mode_drv.rst >> index 86e5867f1b..b2147aad30 100644 >> --- a/doc/guides/prog_guide/poll_mode_drv.rst >> +++ b/doc/guides/prog_guide/poll_mode_drv.rst >> @@ -382,6 +382,8 @@ parameters to those ports. >> -a DBDF,representor=sf[1,3,5] >> -a DBDF,representor=sf[0-1023] >> -a DBDF,representor=sf[0,2-4,7,9-11] >> + -a DBDF,representor=pf1vf0 >> + -a DBDF,representor=pf[0-1]sf[0-127] >> >> Note: PMDs are not required to support the standard device arguments >> and users should consult the relevant PMD documentation to see support >devargs. >> diff --git a/lib/librte_ethdev/ethdev_private.c >> b/lib/librte_ethdev/ethdev_private.c >> index d513f035d0..b9fdbd0f72 100644 >> --- a/lib/librte_ethdev/ethdev_private.c >> +++ b/lib/librte_ethdev/ethdev_private.c >> @@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t >*list, uint16_t *len_list, >> * >> * Representor format: >> * #: range or single number of VF representor - legacy >> - * vf#: VF port representor/s >> - * sf#: SF port representor/s >> + * [pf#]vf#: VF port representor/s >> + * [pf#]sf#: SF port representor/s >> * >> * Examples of #: >> * 2 - single >> @@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char >> *str, void *data) { >> struct rte_eth_devargs *eth_da = data; >> >> + if (str[0] == 'p' && str[1] == 'f') { >> + eth_da->type = RTE_ETH_REPRESENTOR_PF; >> + str += 2; >> + str = rte_eth_devargs_process_list(str, eth_da->ports, >> + ð_da->nb_ports, RTE_MAX_ETHPORTS); > >May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ? Same here, the dim could be different than MAX value. > >> + if (str == NULL) >> + goto err; > >Below we should not allow legacy VF syntax without "vf" prefix. For backward compatibility, default numbers to "vf", otherwise some existing app like OVS that working with VF will break. > >> + } >> if (str[0] == 'v' && str[1] == 'f') { >> eth_da->type = RTE_ETH_REPRESENTOR_VF; >> str += 2; >> @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char *str, >void *data) >> } >> str = rte_eth_devargs_process_list(str, eth_da->representor_ports, >> ð_da->nb_representor_ports, RTE_MAX_ETHPORTS); >> +err: >> if (str == NULL) >> RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); >> return str == NULL ? -1 : 0; >>
On 1/19/21 12:30 PM, Xueming(Steven) Li wrote: > Hi Andrew, > >> -----Original Message----- >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >> Sent: Tuesday, January 19, 2021 4:01 PM >> To: Xueming(Steven) Li <xuemingl@nvidia.com> >> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf Penso >> <asafp@nvidia.com>; NBU-Contact-Thomas Monjalon >> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com> >> Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor >> >> On 1/19/21 10:14 AM, Xueming Li wrote: >>> With Kernel bonding, multiple underlying PFs are bonded, VFs come from >>> different PF, need to identify representor of VFs unambiguously by >>> adding PF index. >>> >>> This patch introduces optional 'pf' section to representor devargs >>> syntax, examples: >>> representor=pf0vf0 - single VF representor >>> representor=pf[0-1]sf[0-1023] - SF representors from 2 PFs >> >> >> Don't we need >> representor=pf3 >> i.e. without VF or sub-function? > > Standalone PF not used by Mellnaox PMD, but should be supported. > Will update. >> >>> >>> >>> Signed-off-by: Xueming Li <xuemingl@nvidia.com> >>> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> >>> Acked-by: Thomas Monjalon <thomas@monjalon.net> >>> --- >>> doc/guides/prog_guide/poll_mode_drv.rst | 2 ++ >>> lib/librte_ethdev/ethdev_private.c | 13 +++++++++++-- >>> 2 files changed, 13 insertions(+), 2 deletions(-) >>> >>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst >>> b/doc/guides/prog_guide/poll_mode_drv.rst >>> index 86e5867f1b..b2147aad30 100644 >>> --- a/doc/guides/prog_guide/poll_mode_drv.rst >>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst >>> @@ -382,6 +382,8 @@ parameters to those ports. >>> -a DBDF,representor=sf[1,3,5] >>> -a DBDF,representor=sf[0-1023] >>> -a DBDF,representor=sf[0,2-4,7,9-11] >>> + -a DBDF,representor=pf1vf0 >>> + -a DBDF,representor=pf[0-1]sf[0-127] >>> >>> Note: PMDs are not required to support the standard device arguments >>> and users should consult the relevant PMD documentation to see support >> devargs. >>> diff --git a/lib/librte_ethdev/ethdev_private.c >>> b/lib/librte_ethdev/ethdev_private.c >>> index d513f035d0..b9fdbd0f72 100644 >>> --- a/lib/librte_ethdev/ethdev_private.c >>> +++ b/lib/librte_ethdev/ethdev_private.c >>> @@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t >> *list, uint16_t *len_list, >>> * >>> * Representor format: >>> * #: range or single number of VF representor - legacy >>> - * vf#: VF port representor/s >>> - * sf#: SF port representor/s >>> + * [pf#]vf#: VF port representor/s >>> + * [pf#]sf#: SF port representor/s >>> * >>> * Examples of #: >>> * 2 - single >>> @@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char >>> *str, void *data) { >>> struct rte_eth_devargs *eth_da = data; >>> >>> + if (str[0] == 'p' && str[1] == 'f') { >>> + eth_da->type = RTE_ETH_REPRESENTOR_PF; >>> + str += 2; >>> + str = rte_eth_devargs_process_list(str, eth_da->ports, >>> + ð_da->nb_ports, RTE_MAX_ETHPORTS); >> >> May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ? > > Same here, the dim could be different than MAX value. Hold on, just for my understanding. The maximum says how many entries could be added to the array. So, why? >> >>> + if (str == NULL) >>> + goto err; >> >> Below we should not allow legacy VF syntax without "vf" prefix. > > For backward compatibility, default numbers to "vf", otherwise some existing > app like OVS that working with VF will break. I mean if new syntax is used (i.e. we have pfX prefix), we must deny legacy syntax for VFs below. >> >>> + } >>> if (str[0] == 'v' && str[1] == 'f') { >>> eth_da->type = RTE_ETH_REPRESENTOR_VF; >>> str += 2; >>> @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char *str, >> void *data) >>> } >>> str = rte_eth_devargs_process_list(str, eth_da->representor_ports, >>> ð_da->nb_representor_ports, RTE_MAX_ETHPORTS); >>> +err: >>> if (str == NULL) >>> RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); >>> return str == NULL ? -1 : 0; >>> >
>-----Original Message----- >From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >Sent: Tuesday, January 19, 2021 5:36 PM >To: Xueming(Steven) Li <xuemingl@nvidia.com> >Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf Penso ><asafp@nvidia.com>; NBU-Contact-Thomas Monjalon ><thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com> >Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor > >On 1/19/21 12:30 PM, Xueming(Steven) Li wrote: >> Hi Andrew, >> >>> -----Original Message----- >>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> >>> Sent: Tuesday, January 19, 2021 4:01 PM >>> To: Xueming(Steven) Li <xuemingl@nvidia.com> >>> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf >>> Penso <asafp@nvidia.com>; NBU-Contact-Thomas Monjalon >>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com> >>> Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor >>> >>> On 1/19/21 10:14 AM, Xueming Li wrote: >>>> With Kernel bonding, multiple underlying PFs are bonded, VFs come >>>> from different PF, need to identify representor of VFs unambiguously >>>> by adding PF index. >>>> >>>> This patch introduces optional 'pf' section to representor devargs >>>> syntax, examples: >>>> representor=pf0vf0 - single VF representor >>>> representor=pf[0-1]sf[0-1023] - SF representors from 2 PFs >>> >>> >>> Don't we need >>> representor=pf3 >>> i.e. without VF or sub-function? >> >> Standalone PF not used by Mellnaox PMD, but should be supported. >> Will update. >>> >>>> >>>> >>>> Signed-off-by: Xueming Li <xuemingl@nvidia.com> >>>> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com> >>>> Acked-by: Thomas Monjalon <thomas@monjalon.net> >>>> --- >>>> doc/guides/prog_guide/poll_mode_drv.rst | 2 ++ >>>> lib/librte_ethdev/ethdev_private.c | 13 +++++++++++-- >>>> 2 files changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/doc/guides/prog_guide/poll_mode_drv.rst >>>> b/doc/guides/prog_guide/poll_mode_drv.rst >>>> index 86e5867f1b..b2147aad30 100644 >>>> --- a/doc/guides/prog_guide/poll_mode_drv.rst >>>> +++ b/doc/guides/prog_guide/poll_mode_drv.rst >>>> @@ -382,6 +382,8 @@ parameters to those ports. >>>> -a DBDF,representor=sf[1,3,5] >>>> -a DBDF,representor=sf[0-1023] >>>> -a DBDF,representor=sf[0,2-4,7,9-11] >>>> + -a DBDF,representor=pf1vf0 >>>> + -a DBDF,representor=pf[0-1]sf[0-127] >>>> >>>> Note: PMDs are not required to support the standard device >>>> arguments and users should consult the relevant PMD documentation >>>> to see support >>> devargs. >>>> diff --git a/lib/librte_ethdev/ethdev_private.c >>>> b/lib/librte_ethdev/ethdev_private.c >>>> index d513f035d0..b9fdbd0f72 100644 >>>> --- a/lib/librte_ethdev/ethdev_private.c >>>> +++ b/lib/librte_ethdev/ethdev_private.c >>>> @@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t >>> *list, uint16_t *len_list, >>>> * >>>> * Representor format: >>>> * #: range or single number of VF representor - legacy >>>> - * vf#: VF port representor/s >>>> - * sf#: SF port representor/s >>>> + * [pf#]vf#: VF port representor/s >>>> + * [pf#]sf#: SF port representor/s >>>> * >>>> * Examples of #: >>>> * 2 - single >>>> @@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char >>>> *str, void *data) { >>>> struct rte_eth_devargs *eth_da = data; >>>> >>>> + if (str[0] == 'p' && str[1] == 'f') { >>>> + eth_da->type = RTE_ETH_REPRESENTOR_PF; >>>> + str += 2; >>>> + str = rte_eth_devargs_process_list(str, eth_da->ports, >>>> + ð_da->nb_ports, RTE_MAX_ETHPORTS); >>> >>> May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ? >> >> Same here, the dim could be different than MAX value. > >Hold on, just for my understanding. The maximum says how many entries >could be added to the array. So, why? Right, will change to RTE_DIM(), thanks! > >>> >>>> + if (str == NULL) >>>> + goto err; >>> >>> Below we should not allow legacy VF syntax without "vf" prefix. >> >> For backward compatibility, default numbers to "vf", otherwise some >> existing app like OVS that working with VF will break. > >I mean if new syntax is used (i.e. we have pfX prefix), we must deny legacy >syntax for VFs below. Correct, will add check, thanks! > >>> >>>> + } >>>> if (str[0] == 'v' && str[1] == 'f') { >>>> eth_da->type = RTE_ETH_REPRESENTOR_VF; >>>> str += 2; >>>> @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char >>>> *str, >>> void *data) >>>> } >>>> str = rte_eth_devargs_process_list(str, eth_da->representor_ports, >>>> ð_da->nb_representor_ports, RTE_MAX_ETHPORTS); >>>> +err: >>>> if (str == NULL) >>>> RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); >>>> return str == NULL ? -1 : 0; >>>> >>
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst index 86e5867f1b..b2147aad30 100644 --- a/doc/guides/prog_guide/poll_mode_drv.rst +++ b/doc/guides/prog_guide/poll_mode_drv.rst @@ -382,6 +382,8 @@ parameters to those ports. -a DBDF,representor=sf[1,3,5] -a DBDF,representor=sf[0-1023] -a DBDF,representor=sf[0,2-4,7,9-11] + -a DBDF,representor=pf1vf0 + -a DBDF,representor=pf[0-1]sf[0-127] Note: PMDs are not required to support the standard device arguments and users should consult the relevant PMD documentation to see support devargs. diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c index d513f035d0..b9fdbd0f72 100644 --- a/lib/librte_ethdev/ethdev_private.c +++ b/lib/librte_ethdev/ethdev_private.c @@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list, * * Representor format: * #: range or single number of VF representor - legacy - * vf#: VF port representor/s - * sf#: SF port representor/s + * [pf#]vf#: VF port representor/s + * [pf#]sf#: SF port representor/s * * Examples of #: * 2 - single @@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data) { struct rte_eth_devargs *eth_da = data; + if (str[0] == 'p' && str[1] == 'f') { + eth_da->type = RTE_ETH_REPRESENTOR_PF; + str += 2; + str = rte_eth_devargs_process_list(str, eth_da->ports, + ð_da->nb_ports, RTE_MAX_ETHPORTS); + if (str == NULL) + goto err; + } if (str[0] == 'v' && str[1] == 'f') { eth_da->type = RTE_ETH_REPRESENTOR_VF; str += 2; @@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data) } str = rte_eth_devargs_process_list(str, eth_da->representor_ports, ð_da->nb_representor_ports, RTE_MAX_ETHPORTS); +err: if (str == NULL) RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); return str == NULL ? -1 : 0;