[dpdk-dev,v5,3/8] ethdev: reserve capability flags for PMD-specific API
Checks
Commit Message
Reserve a Tx capability flag and a Rx capability flag, that can be
used by PMD to define its own capability flags when implementing the
PMD-specific API.
Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
lib/librte_ether/rte_ethdev.h | 2 ++
1 file changed, 2 insertions(+)
Comments
Hi Twei,
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, January 4, 2017 7:22 AM
> To: dev@dpdk.org
> Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
>
> Reserve a Tx capability flag and a Rx capability flag, that can be
> used by PMD to define its own capability flags when implementing the
> PMD-specific API.
>
> Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
> lib/librte_ether/rte_ethdev.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index d465825..8800b39 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -857,6 +857,7 @@ struct rte_eth_conf {
> #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
> #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
> #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
> +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
>
> /**
> * TX offload capabilities of a device.
> @@ -874,6 +875,7 @@ struct rte_eth_conf {
> #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
> #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
> #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
> +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
>
> /**
> * Ethernet device information
> --
> 2.7.4
I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0
is actually a MACSEC for ixgbe?
Another question what to do if you would like to create a bonded device over two devices with different NIC types?
As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities.
Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch?
Konstantin
On Wed, Jan 04, 2017 at 10:21:08PM +0800, Ananyev, Konstantin wrote:
> Hi Twei,
>
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Wednesday, January 4, 2017 7:22 AM
> > To: dev@dpdk.org
> > Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> >
> > Reserve a Tx capability flag and a Rx capability flag, that can be
> > used by PMD to define its own capability flags when implementing the
> > PMD-specific API.
> >
> > Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> > lib/librte_ether/rte_ethdev.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index d465825..8800b39 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -857,6 +857,7 @@ struct rte_eth_conf {
> > #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
> > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
> > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
> > +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
> >
> > /**
> > * TX offload capabilities of a device.
> > @@ -874,6 +875,7 @@ struct rte_eth_conf {
> > #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
> > #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
> > #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
> > +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
> >
> > /**
> > * Ethernet device information
> > --
> > 2.7.4
>
> I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0
> is actually a MACSEC for ixgbe?
Users are not supposed to use DEV_RX_OFFLOAD_RESERVED_0, instead, they
should use the capabilities and the likes defined in rte_pmd_ixgbe.h
where the PMD-specifics APIs are declared:
/**
* If these flags are advertised by the PMD, the NIC supports the MACsec
* offload. The incoming MACsec traffics can be offloaded transparently
* after the MACsec offload is configured correctly by the application.
* And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to
* enable the MACsec offload for the packets to be transmitted.
*/
#define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP DEV_RX_OFFLOAD_RESERVED_0
#define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT DEV_TX_OFFLOAD_RESERVED_0
/**
* This event will occur when the PN counter in a MACsec connection
* reach the exhaustion threshold.
*/
#define RTE_ETH_EVENT_IXGBE_MACSEC RTE_ETH_EVENT_RESERVED_0
/**
* Offload the MACsec. This flag must be set by the application in mbuf
* to enable this offload feature for a packet to be transmitted.
*/
#define PKT_TX_IXGBE_MACSEC PKT_TX_RESERVED_0
PMD-specific APIs can only be used on the corresponding driver/device,
so different PMD can share the same reserved bit to represent different
things when implementing their own PMD-specific APIs.
> Another question what to do if you would like to create a bonded device over two devices with different NIC types?
> As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities.
> Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch?
Those flags are only used in PMD-specific APIs. I don't think we could
use the PMD-specific APIs provided by a certain PMD on a bonded device.
Thanks & regards,
Tiwei Bie
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, January 4, 2017 2:39 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> olivier.matz@6wind.com; thomas.monjalon@6wind.com; Zhang, Helin <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> Xiao W <xiao.w.wang@intel.com>
> Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
>
> On Wed, Jan 04, 2017 at 10:21:08PM +0800, Ananyev, Konstantin wrote:
> > Hi Twei,
> >
> > > -----Original Message-----
> > > From: Bie, Tiwei
> > > Sent: Wednesday, January 4, 2017 7:22 AM
> > > To: dev@dpdk.org
> > > Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Helin
> > > <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > > Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> > >
> > > Reserve a Tx capability flag and a Rx capability flag, that can be
> > > used by PMD to define its own capability flags when implementing the
> > > PMD-specific API.
> > >
> > > Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > ---
> > > lib/librte_ether/rte_ethdev.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > > index d465825..8800b39 100644
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -857,6 +857,7 @@ struct rte_eth_conf {
> > > #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
> > > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
> > > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
> > > +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
> > >
> > > /**
> > > * TX offload capabilities of a device.
> > > @@ -874,6 +875,7 @@ struct rte_eth_conf {
> > > #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
> > > #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
> > > #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
> > > +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
> > >
> > > /**
> > > * Ethernet device information
> > > --
> > > 2.7.4
> >
> > I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0
> > is actually a MACSEC for ixgbe?
>
> Users are not supposed to use DEV_RX_OFFLOAD_RESERVED_0, instead, they
> should use the capabilities and the likes defined in rte_pmd_ixgbe.h
> where the PMD-specifics APIs are declared:
>
> /**
> * If these flags are advertised by the PMD, the NIC supports the MACsec
> * offload. The incoming MACsec traffics can be offloaded transparently
> * after the MACsec offload is configured correctly by the application.
> * And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to
> * enable the MACsec offload for the packets to be transmitted.
> */
> #define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP DEV_RX_OFFLOAD_RESERVED_0
> #define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT DEV_TX_OFFLOAD_RESERVED_0
>
> /**
> * This event will occur when the PN counter in a MACsec connection
> * reach the exhaustion threshold.
> */
> #define RTE_ETH_EVENT_IXGBE_MACSEC RTE_ETH_EVENT_RESERVED_0
>
> /**
> * Offload the MACsec. This flag must be set by the application in mbuf
> * to enable this offload feature for a packet to be transmitted.
> */
> #define PKT_TX_IXGBE_MACSEC PKT_TX_RESERVED_0
>
> PMD-specific APIs can only be used on the corresponding driver/device,
> so different PMD can share the same reserved bit to represent different
> things when implementing their own PMD-specific APIs.
Ok, and why do we need it?
Why we can't just have PKT_TX_MACSEC straightway?
What are you trying to gain here?
Is it just for future opportunity to save an extra bit in mbuf.ol_flags?
I don't think we are short of bits here right now, and we don't consume them exra-fast
to start to worry about it.
Again, if it *really* would be for ixgbe only forever, and y don't want to waste a bit in tx_olflags,
why not to introduce device specific ol_flags in mbuf second cache line?
Probably uint16_t would be enough for that.
>
> > Another question what to do if you would like to create a bonded device over two devices with different NIC types?
> > As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities.
> > Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch?
>
> Those flags are only used in PMD-specific APIs. I don't think we could
> use the PMD-specific APIs provided by a certain PMD on a bonded device.
I understand that.
My question was: suppose user would like to create a bonded device over 2 NICs.
One of them is ixgbe, while other would be some other type.
In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
But it would mean completely different thing.
How bonded device would know that to deal properly?
Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
What does it really mean if there are different NIC types in the system?
The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
Which I suppose, is not always convenient.
Konstantin
On Wed, Jan 04, 2017 at 11:14:25PM +0800, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Wednesday, January 4, 2017 2:39 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Zhang, Helin <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> > Xiao W <xiao.w.wang@intel.com>
> > Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> >
> > On Wed, Jan 04, 2017 at 10:21:08PM +0800, Ananyev, Konstantin wrote:
> > > Hi Twei,
> > >
> > > > -----Original Message-----
> > > > From: Bie, Tiwei
> > > > Sent: Wednesday, January 4, 2017 7:22 AM
> > > > To: dev@dpdk.org
> > > > Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Helin
> > > > <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > > > Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> > > >
> > > > Reserve a Tx capability flag and a Rx capability flag, that can be
> > > > used by PMD to define its own capability flags when implementing the
> > > > PMD-specific API.
> > > >
> > > > Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > > ---
> > > > lib/librte_ether/rte_ethdev.h | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > > > index d465825..8800b39 100644
> > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > @@ -857,6 +857,7 @@ struct rte_eth_conf {
> > > > #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
> > > > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
> > > > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
> > > > +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
> > > >
> > > > /**
> > > > * TX offload capabilities of a device.
> > > > @@ -874,6 +875,7 @@ struct rte_eth_conf {
> > > > #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
> > > > #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
> > > > #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
> > > > +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
> > > >
> > > > /**
> > > > * Ethernet device information
> > > > --
> > > > 2.7.4
> > >
> > > I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0
> > > is actually a MACSEC for ixgbe?
> >
> > Users are not supposed to use DEV_RX_OFFLOAD_RESERVED_0, instead, they
> > should use the capabilities and the likes defined in rte_pmd_ixgbe.h
> > where the PMD-specifics APIs are declared:
> >
> > /**
> > * If these flags are advertised by the PMD, the NIC supports the MACsec
> > * offload. The incoming MACsec traffics can be offloaded transparently
> > * after the MACsec offload is configured correctly by the application.
> > * And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to
> > * enable the MACsec offload for the packets to be transmitted.
> > */
> > #define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP DEV_RX_OFFLOAD_RESERVED_0
> > #define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT DEV_TX_OFFLOAD_RESERVED_0
> >
> > /**
> > * This event will occur when the PN counter in a MACsec connection
> > * reach the exhaustion threshold.
> > */
> > #define RTE_ETH_EVENT_IXGBE_MACSEC RTE_ETH_EVENT_RESERVED_0
> >
> > /**
> > * Offload the MACsec. This flag must be set by the application in mbuf
> > * to enable this offload feature for a packet to be transmitted.
> > */
> > #define PKT_TX_IXGBE_MACSEC PKT_TX_RESERVED_0
> >
> > PMD-specific APIs can only be used on the corresponding driver/device,
> > so different PMD can share the same reserved bit to represent different
> > things when implementing their own PMD-specific APIs.
>
> Ok, and why do we need it?
> Why we can't just have PKT_TX_MACSEC straightway?
> What are you trying to gain here?
> Is it just for future opportunity to save an extra bit in mbuf.ol_flags?
> I don't think we are short of bits here right now, and we don't consume them exra-fast
> to start to worry about it.
> Again, if it *really* would be for ixgbe only forever, and y don't want to waste a bit in tx_olflags,
> why not to introduce device specific ol_flags in mbuf second cache line?
> Probably uint16_t would be enough for that.
>
> >
> > > Another question what to do if you would like to create a bonded device over two devices with different NIC types?
> > > As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities.
> > > Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch?
> >
> > Those flags are only used in PMD-specific APIs. I don't think we could
> > use the PMD-specific APIs provided by a certain PMD on a bonded device.
>
> I understand that.
> My question was: suppose user would like to create a bonded device over 2 NICs.
> One of them is ixgbe, while other would be some other type.
> In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> But it would mean completely different thing.
> How bonded device would know that to deal properly?
>
> Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
>
> Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> What does it really mean if there are different NIC types in the system?
> The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> Which I suppose, is not always convenient.
>
The main purpose is to put the PMD-specific APIs in a separate
namespace instead of mixing the PMD-specific APIs and global APIs
up, and also save the bits in mbuf.ol_flags.
There are other ways to achieve this goal, such as introducing
the PMD specific ol_flags in mbuf second cache line as you said.
I just thought defining some reserved bits seems to be the most
simple way which won't introduce many changes.
What's your suggestions? Should I just revert the changes and
define the generic flags directly?
Thanks & regards,
Tiwei Bie
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Wednesday, January 4, 2017 5:00 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> olivier.matz@6wind.com; thomas.monjalon@6wind.com; Zhang, Helin <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang,
> Xiao W <xiao.w.wang@intel.com>
> Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
>
> On Wed, Jan 04, 2017 at 11:14:25PM +0800, Ananyev, Konstantin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bie, Tiwei
> > > Sent: Wednesday, January 4, 2017 2:39 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: dev@dpdk.org; adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>;
> > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Zhang, Helin <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Wang,
> > > Xiao W <xiao.w.wang@intel.com>
> > > Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> > >
> > > On Wed, Jan 04, 2017 at 10:21:08PM +0800, Ananyev, Konstantin wrote:
> > > > Hi Twei,
> > > >
> > > > > -----Original Message-----
> > > > > From: Bie, Tiwei
> > > > > Sent: Wednesday, January 4, 2017 7:22 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: adrien.mazarguil@6wind.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > > > > olivier.matz@6wind.com; thomas.monjalon@6wind.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Helin
> > > > > <helin.zhang@intel.com>; Dai, Wei <wei.dai@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>
> > > > > Subject: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
> > > > >
> > > > > Reserve a Tx capability flag and a Rx capability flag, that can be
> > > > > used by PMD to define its own capability flags when implementing the
> > > > > PMD-specific API.
> > > > >
> > > > > Suggested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > > > ---
> > > > > lib/librte_ether/rte_ethdev.h | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > > > > index d465825..8800b39 100644
> > > > > --- a/lib/librte_ether/rte_ethdev.h
> > > > > +++ b/lib/librte_ether/rte_ethdev.h
> > > > > @@ -857,6 +857,7 @@ struct rte_eth_conf {
> > > > > #define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
> > > > > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
> > > > > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
> > > > > +#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
> > > > >
> > > > > /**
> > > > > * TX offload capabilities of a device.
> > > > > @@ -874,6 +875,7 @@ struct rte_eth_conf {
> > > > > #define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
> > > > > #define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
> > > > > #define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
> > > > > +#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
> > > > >
> > > > > /**
> > > > > * Ethernet device information
> > > > > --
> > > > > 2.7.4
> > > >
> > > > I am not sure how that supposed to work and how user should know that DEV_RX_OFFLOAD_RESERVED_0
> > > > is actually a MACSEC for ixgbe?
> > >
> > > Users are not supposed to use DEV_RX_OFFLOAD_RESERVED_0, instead, they
> > > should use the capabilities and the likes defined in rte_pmd_ixgbe.h
> > > where the PMD-specifics APIs are declared:
> > >
> > > /**
> > > * If these flags are advertised by the PMD, the NIC supports the MACsec
> > > * offload. The incoming MACsec traffics can be offloaded transparently
> > > * after the MACsec offload is configured correctly by the application.
> > > * And the application can set the PKT_TX_IXGBE_MACSEC flag in mbufs to
> > > * enable the MACsec offload for the packets to be transmitted.
> > > */
> > > #define DEV_RX_OFFLOAD_IXGBE_MACSEC_STRIP DEV_RX_OFFLOAD_RESERVED_0
> > > #define DEV_TX_OFFLOAD_IXGBE_MACSEC_INSERT DEV_TX_OFFLOAD_RESERVED_0
> > >
> > > /**
> > > * This event will occur when the PN counter in a MACsec connection
> > > * reach the exhaustion threshold.
> > > */
> > > #define RTE_ETH_EVENT_IXGBE_MACSEC RTE_ETH_EVENT_RESERVED_0
> > >
> > > /**
> > > * Offload the MACsec. This flag must be set by the application in mbuf
> > > * to enable this offload feature for a packet to be transmitted.
> > > */
> > > #define PKT_TX_IXGBE_MACSEC PKT_TX_RESERVED_0
> > >
> > > PMD-specific APIs can only be used on the corresponding driver/device,
> > > so different PMD can share the same reserved bit to represent different
> > > things when implementing their own PMD-specific APIs.
> >
> > Ok, and why do we need it?
> > Why we can't just have PKT_TX_MACSEC straightway?
> > What are you trying to gain here?
> > Is it just for future opportunity to save an extra bit in mbuf.ol_flags?
> > I don't think we are short of bits here right now, and we don't consume them exra-fast
> > to start to worry about it.
> > Again, if it *really* would be for ixgbe only forever, and y don't want to waste a bit in tx_olflags,
> > why not to introduce device specific ol_flags in mbuf second cache line?
> > Probably uint16_t would be enough for that.
> >
> > >
> > > > Another question what to do if you would like to create a bonded device over two devices with different NIC types?
> > > > As I understand you can end up in situation when DEV_RX_OFFLOAD_RESERVED_0 would mean different capabilities.
> > > > Why not to have this MACSEC capability and ol_flag value as generic ones, as you have them in previous versions of your patch?
> > >
> > > Those flags are only used in PMD-specific APIs. I don't think we could
> > > use the PMD-specific APIs provided by a certain PMD on a bonded device.
> >
> > I understand that.
> > My question was: suppose user would like to create a bonded device over 2 NICs.
> > One of them is ixgbe, while other would be some other type.
> > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > But it would mean completely different thing.
> > How bonded device would know that to deal properly?
> >
> > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> >
> > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > What does it really mean if there are different NIC types in the system?
> > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > Which I suppose, is not always convenient.
> >
>
> The main purpose is to put the PMD-specific APIs in a separate
> namespace instead of mixing the PMD-specific APIs and global APIs
> up, and also save the bits in mbuf.ol_flags.
>
> There are other ways to achieve this goal, such as introducing
> the PMD specific ol_flags in mbuf second cache line as you said.
> I just thought defining some reserved bits seems to be the most
> simple way which won't introduce many changes.
>
> What's your suggestions? Should I just revert the changes and
> define the generic flags directly?
Yes, that would be my preference.
As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
In return there would be no need to consider how to handle all that confusing scenarios in future.
Konstantin
>
> Thanks & regards,
> Tiwei Bie
On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
[...]
> > >
> > > I understand that.
> > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > One of them is ixgbe, while other would be some other type.
> > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > But it would mean completely different thing.
> > > How bonded device would know that to deal properly?
> > >
> > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > >
> > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > What does it really mean if there are different NIC types in the system?
> > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > Which I suppose, is not always convenient.
> > >
> >
> > The main purpose is to put the PMD-specific APIs in a separate
> > namespace instead of mixing the PMD-specific APIs and global APIs
> > up, and also save the bits in mbuf.ol_flags.
> >
> > There are other ways to achieve this goal, such as introducing
> > the PMD specific ol_flags in mbuf second cache line as you said.
> > I just thought defining some reserved bits seems to be the most
> > simple way which won't introduce many changes.
> >
> > What's your suggestions? Should I just revert the changes and
> > define the generic flags directly?
>
> Yes, that would be my preference.
> As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> In return there would be no need to consider how to handle all that confusing scenarios in future.
Okay. I'll update my patches. Thanks a lot for your comments.
Thanks & regards,
Tiwei Bie
On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> [...]
> > > >
> > > > I understand that.
> > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > One of them is ixgbe, while other would be some other type.
> > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > > But it would mean completely different thing.
> > > > How bonded device would know that to deal properly?
> > > >
> > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > >
> > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > What does it really mean if there are different NIC types in the system?
> > > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > > Which I suppose, is not always convenient.
> > > >
> > >
> > > The main purpose is to put the PMD-specific APIs in a separate
> > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > up, and also save the bits in mbuf.ol_flags.
> > >
> > > There are other ways to achieve this goal, such as introducing
> > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > I just thought defining some reserved bits seems to be the most
> > > simple way which won't introduce many changes.
> > >
> > > What's your suggestions? Should I just revert the changes and
> > > define the generic flags directly?
> >
> > Yes, that would be my preference.
> > As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> > In return there would be no need to consider how to handle all that confusing scenarios in future.
>
> Okay. I'll update my patches. Thanks a lot for your comments.
Well, I do not agree with Konstantin (no one saw this coming eh?) and do not
think you need to update your series again.
PMD-specific symbols have nothing to do in the global namespace in my
opinion, they are not versioned and may evolve without notice. Neither
applications nor the bonding PMD can rely on them. That's the trade-off.
Therefore until APIs are made global, the safe compromise is to define
neutral, reserved symbols that any PMD can use to implement their own
temporary APIs for testing purposes. These can be renamed later without
changing their value as long as a single PMD uses them.
On Thu, Jan 05, 2017 at 09:33:22AM +0100, Adrien Mazarguil wrote:
> On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> > [...]
> > > > >
> > > > > I understand that.
> > > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > > One of them is ixgbe, while other would be some other type.
> > > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > > > But it would mean completely different thing.
> > > > > How bonded device would know that to deal properly?
> > > > >
> > > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > > >
> > > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > > What does it really mean if there are different NIC types in the system?
> > > > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > > > Which I suppose, is not always convenient.
> > > > >
> > > >
> > > > The main purpose is to put the PMD-specific APIs in a separate
> > > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > > up, and also save the bits in mbuf.ol_flags.
> > > >
> > > > There are other ways to achieve this goal, such as introducing
> > > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > > I just thought defining some reserved bits seems to be the most
> > > > simple way which won't introduce many changes.
> > > >
> > > > What's your suggestions? Should I just revert the changes and
> > > > define the generic flags directly?
> > >
> > > Yes, that would be my preference.
> > > As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> > > In return there would be no need to consider how to handle all that confusing scenarios in future.
> >
> > Okay. I'll update my patches. Thanks a lot for your comments.
>
> Well, I do not agree with Konstantin (no one saw this coming eh?) and do not
> think you need to update your series again.
>
Hi Adrien,
Thank you very much! :-)
Hi Thomas and Olivier,
I don't have strong opinions here, although I prefer to put
the PMD-specific APIs in a separate namespace. I'd like to
hear or follow your opinions since you are the maintainers
of ethdev and mbuf.
Best regards,
Tiwei Bie
> PMD-specific symbols have nothing to do in the global namespace in my
> opinion, they are not versioned and may evolve without notice. Neither
> applications nor the bonding PMD can rely on them. That's the trade-off.
>
> Therefore until APIs are made global, the safe compromise is to define
> neutral, reserved symbols that any PMD can use to implement their own
> temporary APIs for testing purposes. These can be renamed later without
> changing their value as long as a single PMD uses them.
>
> --
> Adrien Mazarguil
> 6WIND
Hi Adrien,
>
> On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> > [...]
> > > > >
> > > > > I understand that.
> > > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > > One of them is ixgbe, while other would be some other type.
> > > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > > > But it would mean completely different thing.
> > > > > How bonded device would know that to deal properly?
> > > > >
> > > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > > >
> > > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > > What does it really mean if there are different NIC types in the system?
> > > > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > > > Which I suppose, is not always convenient.
> > > > >
> > > >
> > > > The main purpose is to put the PMD-specific APIs in a separate
> > > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > > up, and also save the bits in mbuf.ol_flags.
> > > >
> > > > There are other ways to achieve this goal, such as introducing
> > > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > > I just thought defining some reserved bits seems to be the most
> > > > simple way which won't introduce many changes.
> > > >
> > > > What's your suggestions? Should I just revert the changes and
> > > > define the generic flags directly?
> > >
> > > Yes, that would be my preference.
> > > As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> > > In return there would be no need to consider how to handle all that confusing scenarios in future.
> >
> > Okay. I'll update my patches. Thanks a lot for your comments.
>
> Well, I do not agree with Konstantin (no one saw this coming eh?)
:)
>and do not think you need to update your series again.
>
> PMD-specific symbols have nothing to do in the global namespace in my
> opinion, they are not versioned and may evolve without notice. Neither
> applications nor the bonding PMD can rely on them. That's the trade-off.
Not sure I do understand your reasoning.
For me MACSEC offload is just one of many HW offloads that we support
and should be treated in exactly the same way.
Applications should be able to use it in a transparent and reliable way,
not only under some limited conditions.
Otherwise what is the point to introduce it at all?
Yes, right now it is supported only by ixgbe PMD, but why that should be the
reason to treat is as second-class citizen?
Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.
>
> Therefore until APIs are made global, the safe compromise is to define
> neutral, reserved symbols that any PMD can use to implement their own
> temporary APIs for testing purposes. These can be renamed later without
> changing their value as long as a single PMD uses them.
Ok, so what we'll gain by introducing PKT_TX_RESERVED instead of PKT_TX_MACSEC?
As I said in my previous mail the redefinition for the same ol_flag bit (and dev capabilities)
by different PMD might create a lot of confusion in future.
Does the potential saving of 1 bit really worth it?
Konstantin
Hi Konstantin,
On Thu, Jan 05, 2017 at 11:32:38AM +0000, Ananyev, Konstantin wrote:
> Hi Adrien,
>
> >
> > On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> > > [...]
> > > > > >
> > > > > > I understand that.
> > > > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > > > One of them is ixgbe, while other would be some other type.
> > > > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > > > > But it would mean completely different thing.
> > > > > > How bonded device would know that to deal properly?
> > > > > >
> > > > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > > > >
> > > > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > > > What does it really mean if there are different NIC types in the system?
> > > > > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > > > > Which I suppose, is not always convenient.
> > > > > >
> > > > >
> > > > > The main purpose is to put the PMD-specific APIs in a separate
> > > > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > > > up, and also save the bits in mbuf.ol_flags.
> > > > >
> > > > > There are other ways to achieve this goal, such as introducing
> > > > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > > > I just thought defining some reserved bits seems to be the most
> > > > > simple way which won't introduce many changes.
> > > > >
> > > > > What's your suggestions? Should I just revert the changes and
> > > > > define the generic flags directly?
> > > >
> > > > Yes, that would be my preference.
> > > > As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> > > > In return there would be no need to consider how to handle all that confusing scenarios in future.
> > >
> > > Okay. I'll update my patches. Thanks a lot for your comments.
> >
> > Well, I do not agree with Konstantin (no one saw this coming eh?)
>
> :)
>
> >and do not think you need to update your series again.
> >
> > PMD-specific symbols have nothing to do in the global namespace in my
> > opinion, they are not versioned and may evolve without notice. Neither
> > applications nor the bonding PMD can rely on them. That's the trade-off.
>
> Not sure I do understand your reasoning.
> For me MACSEC offload is just one of many HW offloads that we support
> and should be treated in exactly the same way.
> Applications should be able to use it in a transparent and reliable way,
> not only under some limited conditions.
> Otherwise what is the point to introduce it at all?
Well my first reply to this thread was asking why isn't the whole API global
from the start then?
Given there are valid reasons for it not to and no plan to make it so in the
near future, applications must be aware that they are including
rte_pmd_ixgbe.h to use it. That in itself is a limiting condition, right?
> Yes, right now it is supported only by ixgbe PMD, but why that should be the
> reason to treat is as second-class citizen?
> Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.
You are right about PKT_TX_TUNNEL_*, however these flags exist on their own
and are not tied to any API function calls, unlike in this series where
PKT_TX_MACSEC can only be used if the DEV_TX_OFFLOAD_MACSEC_INSERT
capability is present and the whole thing was configured through
rte_pmd_ixgbe_macsec_*() calls after including rte_pmd_ixgbe.h.
To be clear it is not about MACsec per se (as a standardized protocol, I
think related definitions for offloads have their place), but it has to do
with the fact that the rest of the API is PMD-specific and there is a
dependency between them.
> > Therefore until APIs are made global, the safe compromise is to define
> > neutral, reserved symbols that any PMD can use to implement their own
> > temporary APIs for testing purposes. These can be renamed later without
> > changing their value as long as a single PMD uses them.
>
> Ok, so what we'll gain by introducing PKT_TX_RESERVED instead of PKT_TX_MACSEC?
> As I said in my previous mail the redefinition for the same ol_flag bit (and dev capabilities)
> by different PMD might create a lot of confusion in future.
> Does the potential saving of 1 bit really worth it?
That is one benefit, but my point is mainly to keep applications aware that
they are using an API defined by a single PMD, which may be temporary and
whose symbols are not versioned.
Consider this:
rte_mbuf.h:
#define PKT_TX_RESERVED_0 (1 << 42)
rte_pmd_ixgbe.h:
#define PKT_TX_MACSEC PKT_TX_RESERVED_0
That way, applications have to get the PKT_TX_MACSEC definition where the
rest of the API is also defined.
Other PMDs may reuse PKT_TX_RESERVED_0 and other reserved flags to implement
their own experimental APIs.
Applications and the bonding PMD can easily be made aware that such reserved
flags cannot be shared between ports unless they know what the underlying
PMD is, which is already a requirement to use this API in the first place
(for instance, calling rte_pmd_ixgbe_macsec_*() functions with another
vendor's port_id may crash the application).
So the idea if/when the API is made global is to rename PKT_TX_RESERVED_0 to
PKT_TX_MACSEC and keep its original value.
If other PMDs also implemented PKT_TX_RESERVED_0 in the meantime, it is
redefined using a different value. If there is no room left to do so, these
PMDs are out of luck I guess, and their specific API is disabled/removed
until something gets re-designed.
How about this?
Hi Adrien,
>
> Hi Konstantin,
>
> On Thu, Jan 05, 2017 at 11:32:38AM +0000, Ananyev, Konstantin wrote:
> > Hi Adrien,
> >
> > >
> > > On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > > > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> > > > [...]
> > > > > > >
> > > > > > > I understand that.
> > > > > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > > > > One of them is ixgbe, while other would be some other type.
> > > > > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0 bit as set.
> > > > > > > But it would mean completely different thing.
> > > > > > > How bonded device would know that to deal properly?
> > > > > > >
> > > > > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > > > > >
> > > > > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > > > > What does it really mean if there are different NIC types in the system?
> > > > > > > The only way to answer that question, as I can see, is to keep track from what NIC that packet was received.
> > > > > > > Which I suppose, is not always convenient.
> > > > > > >
> > > > > >
> > > > > > The main purpose is to put the PMD-specific APIs in a separate
> > > > > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > > > > up, and also save the bits in mbuf.ol_flags.
> > > > > >
> > > > > > There are other ways to achieve this goal, such as introducing
> > > > > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > > > > I just thought defining some reserved bits seems to be the most
> > > > > > simple way which won't introduce many changes.
> > > > > >
> > > > > > What's your suggestions? Should I just revert the changes and
> > > > > > define the generic flags directly?
> > > > >
> > > > > Yes, that would be my preference.
> > > > > As I said above - spending extra bit in ol_flags doesn't look like a big problem to me.
> > > > > In return there would be no need to consider how to handle all that confusing scenarios in future.
> > > >
> > > > Okay. I'll update my patches. Thanks a lot for your comments.
> > >
> > > Well, I do not agree with Konstantin (no one saw this coming eh?)
> >
> > :)
> >
> > >and do not think you need to update your series again.
> > >
> > > PMD-specific symbols have nothing to do in the global namespace in my
> > > opinion, they are not versioned and may evolve without notice. Neither
> > > applications nor the bonding PMD can rely on them. That's the trade-off.
> >
> > Not sure I do understand your reasoning.
> > For me MACSEC offload is just one of many HW offloads that we support
> > and should be treated in exactly the same way.
> > Applications should be able to use it in a transparent and reliable way,
> > not only under some limited conditions.
> > Otherwise what is the point to introduce it at all?
>
> Well my first reply to this thread was asking why isn't the whole API global
> from the start then?
That's good question, and my preference would always be to have the
API to configure this feature as generic one.
I guess the main reason why it is not right now we don't reach an agreement
how this API should look like:
http://dpdk.org/ml/archives/dev/2016-September/047810.html
But I'll leave it to the author to provide the real reason here.
>
> Given there are valid reasons for it not to and no plan to make it so in the
> near future, applications must be aware that they are including
> rte_pmd_ixgbe.h to use it. That in itself is a limiting condition, right?
Yes, it is definitely a limiting factor.
Though even if API to configure device to use macsec would be PMD specific right now,
The API to query that capability and the API to use it at datapath (mbuf.ol_flags) still
can be (and I think should be) device independent and transparent to use.
>
> > Yes, right now it is supported only by ixgbe PMD, but why that should be the
> > reason to treat is as second-class citizen?
> > Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.
>
> You are right about PKT_TX_TUNNEL_*, however these flags exist on their own
> and are not tied to any API function calls, unlike in this series where
> PKT_TX_MACSEC can only be used if the DEV_TX_OFFLOAD_MACSEC_INSERT
> capability is present
I don't think PKT_TX_TUNNEL_* 'exists on its own'.
To use it well behaving app have to:
1) Query that device does provide that capability: DEV_TX_OFFLOAD_*_TNL_TSO
2) configure PMD( & device) to use that capability
3) use that offload at run-time TX code (mb->ol_flags |= ...; mb->tx_offload = ...)
For PKT_TX_TUNNEL_* 2) is pretty simple - user just need to make sure
that full-featured TX function will be selected:
txconf.txq_flags = 0; ...; rte_eth_tx_queue_setup(..., &txconf);
For TX_MACSEC, as I understand 2) will be more complicated and
right now is PMD specific, but anyway the main pattern remains the same.
So at least 1) and 3) could be kept device neutral.
>and the whole thing was configured through
> rte_pmd_ixgbe_macsec_*() calls after including rte_pmd_ixgbe.h.
>
> To be clear it is not about MACsec per se (as a standardized protocol, I
> think related definitions for offloads have their place), but it has to do
> with the fact that the rest of the API is PMD-specific and there is a
> dependency between them.
>
> > > Therefore until APIs are made global, the safe compromise is to define
> > > neutral, reserved symbols that any PMD can use to implement their own
> > > temporary APIs for testing purposes. These can be renamed later without
> > > changing their value as long as a single PMD uses them.
> >
> > Ok, so what we'll gain by introducing PKT_TX_RESERVED instead of PKT_TX_MACSEC?
> > As I said in my previous mail the redefinition for the same ol_flag bit (and dev capabilities)
> > by different PMD might create a lot of confusion in future.
> > Does the potential saving of 1 bit really worth it?
>
> That is one benefit, but my point is mainly to keep applications aware that
> they are using an API defined by a single PMD, which may be temporary and
> whose symbols are not versioned.
As applications have to use PMD specific functions to configure it they definitely are aware.
>
> Consider this:
>
> rte_mbuf.h:
>
> #define PKT_TX_RESERVED_0 (1 << 42)
>
> rte_pmd_ixgbe.h:
>
> #define PKT_TX_MACSEC PKT_TX_RESERVED_0
>
> That way, applications have to get the PKT_TX_MACSEC definition where the
> rest of the API is also defined.
>
> Other PMDs may reuse PKT_TX_RESERVED_0 and other reserved flags to implement
> their own experimental APIs.
That's the main thing I am opposed to.
I think that by allowing PMD to redefine meaning of
mbuf.ol_flags and dev_info.(rx| tx)_offload_capa
we just asking for trouble.
Let say tomorrow, i40e will redefine DEV_TX_OFFLOAD_RESERVED_0 and PKT_TX_RESERVED_0
to implement new specific TX offload (PKT_TX_FEATURE_X).
Now let say we have an application that works over both ixgbe and i40e
and would like to use both TX_MACSEC and TC_FEATURE_X offloads whenever they are available.
As I can see, with the approach you proposed the only way for the application to make it
is to support 2 different TX code paths (or at least some parts of it).
To me that way looks inconvenient to the users and source of future troubles.
Same for RX: somewhere at upper layer user got a packet with PKT_RX_RESERVED_0 set.
What does it really mean if there are different NIC types in the system?
>
> Applications and the bonding PMD can easily be made aware that such reserved
> flags cannot be shared between ports unless they know what the underlying
> PMD is, which is already a requirement to use this API in the first place
> (for instance, calling rte_pmd_ixgbe_macsec_*() functions with another
> vendor's port_id may crash the application).
I am talking about that code:
rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
{
...
/* Take the first dev's offload capabilities */
internals->rx_offload_capa = dev_info.rx_offload_capa;
internals->tx_offload_capa = dev_info.tx_offload_capa;
...
internals->rx_offload_capa &= dev_info.rx_offload_capa;
internals->tx_offload_capa &= dev_info.tx_offload_capa;
Obviously with what you are suggesting it is not valid any more.
Bonded device need to support a MASK of all device reserved offloads to exclude
them from common subset.
Any user app(/lib) that does similar thing would also have to be changed.
>
> So the idea if/when the API is made global is to rename PKT_TX_RESERVED_0 to
> PKT_TX_MACSEC and keep its original value.
>
> If other PMDs also implemented PKT_TX_RESERVED_0 in the meantime, it is
> redefined using a different value. If there is no room left to do so, these
> PMDs are out of luck I guess, and their specific API is disabled/removed
> until something gets re-designed.
>
> How about this?
I still think that we shouldn't allow PMDs to redefine mbuf.olflags and
dev_info.(rx|tx)_offload_capa.
See above for my reasons.
Konstantin
On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin wrote:
> Hi Adrien,
>
> >
> > Hi Konstantin,
> >
> > On Thu, Jan 05, 2017 at 11:32:38AM +0000, Ananyev, Konstantin wrote:
> > > Hi Adrien,
> > >
> > > >
> > > > On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > > > > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
[...]
> > Well my first reply to this thread was asking why isn't the whole API global
> > from the start then?
>
> That's good question, and my preference would always be to have the
> API to configure this feature as generic one.
> I guess the main reason why it is not right now we don't reach an agreement
> how this API should look like:
> http://dpdk.org/ml/archives/dev/2016-September/047810.html
> But I'll leave it to the author to provide the real reason here.
>
Yes, currently this work just provided a thin layer over 82599's
hardware MACsec offload support to allow users configure 82599's
MACsec offload engine. The current API may be too specific and may
need a rework to be used with other NICs.
> >
> > Given there are valid reasons for it not to and no plan to make it so in the
> > near future, applications must be aware that they are including
> > rte_pmd_ixgbe.h to use it. That in itself is a limiting condition, right?
>
> Yes, it is definitely a limiting factor.
> Though even if API to configure device to use macsec would be PMD specific right now,
> The API to query that capability and the API to use it at datapath (mbuf.ol_flags) still
> can be (and I think should be) device independent and transparent to use.
>
> >
> > > Yes, right now it is supported only by ixgbe PMD, but why that should be the
> > > reason to treat is as second-class citizen?
> > > Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.
> >
> > You are right about PKT_TX_TUNNEL_*, however these flags exist on their own
> > and are not tied to any API function calls, unlike in this series where
> > PKT_TX_MACSEC can only be used if the DEV_TX_OFFLOAD_MACSEC_INSERT
> > capability is present
>
> I don't think PKT_TX_TUNNEL_* 'exists on its own'.
> To use it well behaving app have to:
> 1) Query that device does provide that capability: DEV_TX_OFFLOAD_*_TNL_TSO
> 2) configure PMD( & device) to use that capability
> 3) use that offload at run-time TX code (mb->ol_flags |= ...; mb->tx_offload = ...)
>
> For PKT_TX_TUNNEL_* 2) is pretty simple - user just need to make sure
> that full-featured TX function will be selected:
> txconf.txq_flags = 0; ...; rte_eth_tx_queue_setup(..., &txconf);
>
> For TX_MACSEC, as I understand 2) will be more complicated and
> right now is PMD specific, but anyway the main pattern remains the same.
> So at least 1) and 3) could be kept device neutral.
>
Yes, the PMD-specific APIs focus on the 2nd step:
2) configure PMD( & device) to use that capability
The other parts (querying the capabilities, using the offload
in the datapath, registering the callback, getting the statistics
via xstats) are done in generic way.
Best regards,
Tiwei Bie
2017-01-09 11:57, Tiwei Bie:
> On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin wrote:
> > > Well my first reply to this thread was asking why isn't the whole API global
> > > from the start then?
> >
> > That's good question, and my preference would always be to have the
> > API to configure this feature as generic one.
> > I guess the main reason why it is not right now we don't reach an agreement
> > how this API should look like:
> > http://dpdk.org/ml/archives/dev/2016-September/047810.html
> > But I'll leave it to the author to provide the real reason here.
>
> Yes, currently this work just provided a thin layer over 82599's
> hardware MACsec offload support to allow users configure 82599's
> MACsec offload engine. The current API may be too specific and may
> need a rework to be used with other NICs.
I think it is a really good approach to start such API privately in a driver.
It will give us more time and experience to design a proper generic API.
Regarding the mbuf flag, it looks straight-forward, and as it is IEEE
standardized, I do not see any objection to add it now.
However, I will wait for the approval of Olivier - as maintainer of mbuf.
Hi Konstantin,
On Sun, Jan 08, 2017 at 12:39:55PM +0000, Ananyev, Konstantin wrote:
>
> Hi Adrien,
>
> >
> > Hi Konstantin,
> >
> > On Thu, Jan 05, 2017 at 11:32:38AM +0000, Ananyev, Konstantin wrote:
> > > Hi Adrien,
[...]
> > > > PMD-specific symbols have nothing to do in the global namespace in my
> > > > opinion, they are not versioned and may evolve without notice. Neither
> > > > applications nor the bonding PMD can rely on them. That's the trade-off.
> > >
> > > Not sure I do understand your reasoning.
> > > For me MACSEC offload is just one of many HW offloads that we support
> > > and should be treated in exactly the same way.
> > > Applications should be able to use it in a transparent and reliable way,
> > > not only under some limited conditions.
> > > Otherwise what is the point to introduce it at all?
> >
> > Well my first reply to this thread was asking why isn't the whole API global
> > from the start then?
>
> That's good question, and my preference would always be to have the
> API to configure this feature as generic one.
> I guess the main reason why it is not right now we don't reach an agreement
> how this API should look like:
> http://dpdk.org/ml/archives/dev/2016-September/047810.html
> But I'll leave it to the author to provide the real reason here.
>
> > Given there are valid reasons for it not to and no plan to make it so in the
> > near future, applications must be aware that they are including
> > rte_pmd_ixgbe.h to use it. That in itself is a limiting condition, right?
>
> Yes, it is definitely a limiting factor.
> Though even if API to configure device to use macsec would be PMD specific right now,
> The API to query that capability and the API to use it at datapath (mbuf.ol_flags) still
> can be (and I think should be) device independent and transparent to use.
With RESERVED flags, what is global and transparent is the fact the mbuf or
device features in question are PMD-specific and some extra knowledge about
the underlying port is necessary to handle them. I think that could be
useful to applications in order to get the necessary precautions.
> > > Yes, right now it is supported only by ixgbe PMD, but why that should be the
> > > reason to treat is as second-class citizen?
> > > Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.
> >
> > You are right about PKT_TX_TUNNEL_*, however these flags exist on their own
> > and are not tied to any API function calls, unlike in this series where
> > PKT_TX_MACSEC can only be used if the DEV_TX_OFFLOAD_MACSEC_INSERT
> > capability is present
>
> I don't think PKT_TX_TUNNEL_* 'exists on its own'.
> To use it well behaving app have to:
> 1) Query that device does provide that capability: DEV_TX_OFFLOAD_*_TNL_TSO
> 2) configure PMD( & device) to use that capability
> 3) use that offload at run-time TX code (mb->ol_flags |= ...; mb->tx_offload = ...)
>
> For PKT_TX_TUNNEL_* 2) is pretty simple - user just need to make sure
> that full-featured TX function will be selected:
> txconf.txq_flags = 0; ...; rte_eth_tx_queue_setup(..., &txconf);
Pretty much like any other offloads. Any PMD could implement them as well
without exposing additional callbacks.
> For TX_MACSEC, as I understand 2) will be more complicated and
> right now is PMD specific, but anyway the main pattern remains the same.
> So at least 1) and 3) could be kept device neutral.
Yes, however this discussion is precisely because I think it could be a
problem that this API "right now is PMD specific", particularly because it
will remain that way.
> >and the whole thing was configured through
> > rte_pmd_ixgbe_macsec_*() calls after including rte_pmd_ixgbe.h.
> >
> > To be clear it is not about MACsec per se (as a standardized protocol, I
> > think related definitions for offloads have their place), but it has to do
> > with the fact that the rest of the API is PMD-specific and there is a
> > dependency between them.
> >
> > > > Therefore until APIs are made global, the safe compromise is to define
> > > > neutral, reserved symbols that any PMD can use to implement their own
> > > > temporary APIs for testing purposes. These can be renamed later without
> > > > changing their value as long as a single PMD uses them.
> > >
> > > Ok, so what we'll gain by introducing PKT_TX_RESERVED instead of PKT_TX_MACSEC?
> > > As I said in my previous mail the redefinition for the same ol_flag bit (and dev capabilities)
> > > by different PMD might create a lot of confusion in future.
> > > Does the potential saving of 1 bit really worth it?
> >
> > That is one benefit, but my point is mainly to keep applications aware that
> > they are using an API defined by a single PMD, which may be temporary and
> > whose symbols are not versioned.
>
> As applications have to use PMD specific functions to configure it they definitely are aware.
Therefore they also know if they are only running on top of ixgbe adapters
of a mix with something else. By choosing the PMD-specific path, they know
what they are getting into.
> > Consider this:
> >
> > rte_mbuf.h:
> >
> > #define PKT_TX_RESERVED_0 (1 << 42)
> >
> > rte_pmd_ixgbe.h:
> >
> > #define PKT_TX_MACSEC PKT_TX_RESERVED_0
> >
> > That way, applications have to get the PKT_TX_MACSEC definition where the
> > rest of the API is also defined.
> >
> > Other PMDs may reuse PKT_TX_RESERVED_0 and other reserved flags to implement
> > their own experimental APIs.
>
> That's the main thing I am opposed to.
> I think that by allowing PMD to redefine meaning of
> mbuf.ol_flags and dev_info.(rx| tx)_offload_capa
> we just asking for trouble.
>
> Let say tomorrow, i40e will redefine DEV_TX_OFFLOAD_RESERVED_0 and PKT_TX_RESERVED_0
> to implement new specific TX offload (PKT_TX_FEATURE_X).
> Now let say we have an application that works over both ixgbe and i40e
> and would like to use both TX_MACSEC and TC_FEATURE_X offloads whenever they are available.
> As I can see, with the approach you proposed the only way for the application to make it
> is to support 2 different TX code paths (or at least some parts of it).
> To me that way looks inconvenient to the users and source of future troubles.
Remember applications still have to clear PKT_TX_MACSEC and other flags on
ports that do not implement them, just like when the related offload is not
configured even if supported. Whichever approach is taken, applications have
to be careful and need a few extra checks in their TX path. There is no
extra cost involved compared to existing offloads.
> Same for RX: somewhere at upper layer user got a packet with PKT_RX_RESERVED_0 set.
> What does it really mean if there are different NIC types in the system?
For RX, I agree things are more complicated, applications would have to know
what a given flag means from a particular port. We could define several
RESERVED_X flags that do not overlap on a case basis, so at least
applications know they are dealing with PMD-specific flags.
> > Applications and the bonding PMD can easily be made aware that such reserved
> > flags cannot be shared between ports unless they know what the underlying
> > PMD is, which is already a requirement to use this API in the first place
> > (for instance, calling rte_pmd_ixgbe_macsec_*() functions with another
> > vendor's port_id may crash the application).
>
> I am talking about that code:
> rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
> {
> ...
> /* Take the first dev's offload capabilities */
> internals->rx_offload_capa = dev_info.rx_offload_capa;
> internals->tx_offload_capa = dev_info.tx_offload_capa;
> ...
> internals->rx_offload_capa &= dev_info.rx_offload_capa;
> internals->tx_offload_capa &= dev_info.tx_offload_capa;
>
> Obviously with what you are suggesting it is not valid any more.
> Bonded device need to support a MASK of all device reserved offloads to exclude
> them from common subset.
> Any user app(/lib) that does similar thing would also have to be changed.
Why can't they clear RESERVED flags as well? We just have to document the
expected behavior. Even if the bonding PMD does not, applications that do
not use those flags (since they are reserved) should not be impacted.
Because the bonding PMD won't forward PMD-specific API calls, I think it's
safer to clear them anyway.
By the way, how do you handle the case where the bonding PMD exposes the
MACSEC feature and as a result the application tries to configure MACSEC
support on the bonding port_id? How is the resulting crash documented in a
generic fashion?
> > So the idea if/when the API is made global is to rename PKT_TX_RESERVED_0 to
> > PKT_TX_MACSEC and keep its original value.
> >
> > If other PMDs also implemented PKT_TX_RESERVED_0 in the meantime, it is
> > redefined using a different value. If there is no room left to do so, these
> > PMDs are out of luck I guess, and their specific API is disabled/removed
> > until something gets re-designed.
> >
> > How about this?
>
> I still think that we shouldn't allow PMDs to redefine mbuf.olflags and
> dev_info.(rx|tx)_offload_capa.
> See above for my reasons.
I'm still not comfortable with partially global/specific APIs such as this,
because we assume applications are fully aware of the underlying port in
order to use them, while at the same time the related flags are part of the
global namespace, I find it confusing.
If application developers have no problem with that (so far they haven't
commented on this topic, which means they likely agree to go with anything),
I have no reason left to go against the majority.
On Mon, Jan 09, 2017 at 12:26:53PM +0100, Thomas Monjalon wrote:
> 2017-01-09 11:57, Tiwei Bie:
> > On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin wrote:
> > > > Well my first reply to this thread was asking why isn't the whole API global
> > > > from the start then?
> > >
> > > That's good question, and my preference would always be to have the
> > > API to configure this feature as generic one.
> > > I guess the main reason why it is not right now we don't reach an agreement
> > > how this API should look like:
> > > http://dpdk.org/ml/archives/dev/2016-September/047810.html
> > > But I'll leave it to the author to provide the real reason here.
> >
> > Yes, currently this work just provided a thin layer over 82599's
> > hardware MACsec offload support to allow users configure 82599's
> > MACsec offload engine. The current API may be too specific and may
> > need a rework to be used with other NICs.
>
> I think it is a really good approach to start such API privately in a driver.
> It will give us more time and experience to design a proper generic API.
>
> Regarding the mbuf flag, it looks straight-forward, and as it is IEEE
> standardized, I do not see any objection to add it now.
> However, I will wait for the approval of Olivier - as maintainer of mbuf.
>
I see. Thank you very much for your comments! :-)
Best regards,
Tiwei Bie
Hi Tiwei, Hi Thomas,
On Mon, 09 Jan 2017 12:26:53 +0100, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2017-01-09 11:57, Tiwei Bie:
> > On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin
> > wrote:
> > > > Well my first reply to this thread was asking why isn't the
> > > > whole API global from the start then?
> > >
> > > That's good question, and my preference would always be to have
> > > the API to configure this feature as generic one.
> > > I guess the main reason why it is not right now we don't reach an
> > > agreement how this API should look like:
> > > http://dpdk.org/ml/archives/dev/2016-September/047810.html
> > > But I'll leave it to the author to provide the real reason
> > > here.
> >
> > Yes, currently this work just provided a thin layer over 82599's
> > hardware MACsec offload support to allow users configure 82599's
> > MACsec offload engine. The current API may be too specific and may
> > need a rework to be used with other NICs.
>
> I think it is a really good approach to start such API privately in a
> driver. It will give us more time and experience to design a proper
> generic API.
>
> Regarding the mbuf flag, it looks straight-forward, and as it is IEEE
> standardized, I do not see any objection to add it now.
> However, I will wait for the approval of Olivier - as maintainer of
> mbuf.
>
Generally speaking, we have to be careful when introducing new mbuf
flags, since we don't have so much of them (~25 remaining out of 64,
which mean we may run out of them in 3-4 years).
In this particular case, despite the flag is added for an ixgbe-specific
API, when MACsec will be implemented on another PMD, the exact same
flag would also be needed. That's the same for the ethdev capability
flag. Moreover, as Thomas stated, it's a standardized protocol so it's
legitimate to have it included in rte_mbuf.h.
For me, having PMD-specific APIs for a new feature is not a problem,
but I think we should try to have a generic API as soon as the feature
is supported by several PMDs.
Regards,
Olivier
On Wed, Jan 11, 2017 at 06:32:48PM +0100, Olivier MATZ wrote:
> Hi Tiwei, Hi Thomas,
>
> On Mon, 09 Jan 2017 12:26:53 +0100, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
> > 2017-01-09 11:57, Tiwei Bie:
> > > On Sun, Jan 08, 2017 at 08:39:55PM +0800, Ananyev, Konstantin
> > > wrote:
> > > > > Well my first reply to this thread was asking why isn't the
> > > > > whole API global from the start then?
> > > >
> > > > That's good question, and my preference would always be to have
> > > > the API to configure this feature as generic one.
> > > > I guess the main reason why it is not right now we don't reach an
> > > > agreement how this API should look like:
> > > > http://dpdk.org/ml/archives/dev/2016-September/047810.html
> > > > But I'll leave it to the author to provide the real reason
> > > > here.
> > >
> > > Yes, currently this work just provided a thin layer over 82599's
> > > hardware MACsec offload support to allow users configure 82599's
> > > MACsec offload engine. The current API may be too specific and may
> > > need a rework to be used with other NICs.
> >
> > I think it is a really good approach to start such API privately in a
> > driver. It will give us more time and experience to design a proper
> > generic API.
> >
> > Regarding the mbuf flag, it looks straight-forward, and as it is IEEE
> > standardized, I do not see any objection to add it now.
> > However, I will wait for the approval of Olivier - as maintainer of
> > mbuf.
> >
>
> Generally speaking, we have to be careful when introducing new mbuf
> flags, since we don't have so much of them (~25 remaining out of 64,
> which mean we may run out of them in 3-4 years).
>
> In this particular case, despite the flag is added for an ixgbe-specific
> API, when MACsec will be implemented on another PMD, the exact same
> flag would also be needed. That's the same for the ethdev capability
> flag. Moreover, as Thomas stated, it's a standardized protocol so it's
> legitimate to have it included in rte_mbuf.h.
>
> For me, having PMD-specific APIs for a new feature is not a problem,
> but I think we should try to have a generic API as soon as the feature
> is supported by several PMDs.
>
Sure! Thank you very much!
Best regards,
Tiwei Bie
On Wed, Jan 11, 2017 at 06:32:48PM +0100, Olivier MATZ wrote:
> Generally speaking, we have to be careful when introducing new mbuf
> flags, since we don't have so much of them (~25 remaining out of 64,
> which mean we may run out of them in 3-4 years).
>
> In this particular case, despite the flag is added for an ixgbe-specific
> API, when MACsec will be implemented on another PMD, the exact same
> flag would also be needed. That's the same for the ethdev capability
> flag. Moreover, as Thomas stated, it's a standardized protocol so it's
> legitimate to have it included in rte_mbuf.h.
>
> For me, having PMD-specific APIs for a new feature is not a problem,
> but I think we should try to have a generic API as soon as the feature
> is supported by several PMDs.
JFYI, here is how the virtio handle the feature bits. It basically
just divides it to two parts.
From virtio 1.0 spec (2.2 Feature Bits):
Feature bits are allocated as follows:
- 0 to 23 Feature bits for the specific device type
- 24 to 32 Feature bits reserved for extensions to the queue and feature
negotiation mechanisms
- 33 and above Feature bits reserved for future extensions.
Note: For example, feature bit 0 for a network device (i.e. Device ID 1)
indicates that the device supports checksumming of packets.
--yliu
@@ -857,6 +857,7 @@ struct rte_eth_conf {
#define DEV_RX_OFFLOAD_TCP_LRO 0x00000010
#define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020
#define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040
+#define DEV_RX_OFFLOAD_RESERVED_0 0x00000080 /**< Used for PMD-specific API. */
/**
* TX offload capabilities of a device.
@@ -874,6 +875,7 @@ struct rte_eth_conf {
#define DEV_TX_OFFLOAD_GRE_TNL_TSO 0x00000400 /**< Used for tunneling packet. */
#define DEV_TX_OFFLOAD_IPIP_TNL_TSO 0x00000800 /**< Used for tunneling packet. */
#define DEV_TX_OFFLOAD_GENEVE_TNL_TSO 0x00001000 /**< Used for tunneling packet. */
+#define DEV_TX_OFFLOAD_RESERVED_0 0x00002000 /**< Used for PMD-specific API. */
/**
* Ethernet device information