[10/15] net/octeontx2: switch timestamp to dynamic mbuf field
Checks
Commit Message
The mbuf timestamp is moved to a dynamic field
in order to allow removal of the deprecated static field.
The related mbuf flag is also replaced.
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
drivers/net/octeontx2/otx2_ethdev.c | 33 +++++++++++++++++++++++++++++
drivers/net/octeontx2/otx2_rx.h | 19 ++++++++++++++---
drivers/net/octeontx2/version.map | 7 ++++++
3 files changed, 56 insertions(+), 3 deletions(-)
Comments
On 10/29/20 12:27 PM, Thomas Monjalon wrote:
> The mbuf timestamp is moved to a dynamic field
> in order to allow removal of the deprecated static field.
> The related mbuf flag is also replaced.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> drivers/net/octeontx2/otx2_ethdev.c | 33 +++++++++++++++++++++++++++++
> drivers/net/octeontx2/otx2_rx.h | 19 ++++++++++++++---
> drivers/net/octeontx2/version.map | 7 ++++++
> 3 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
> index cfb733a4b5..ad95219438 100644
> --- a/drivers/net/octeontx2/otx2_ethdev.c
> +++ b/drivers/net/octeontx2/otx2_ethdev.c
> @@ -4,6 +4,7 @@
>
> #include <inttypes.h>
>
> +#include <rte_bitops.h>
> #include <rte_ethdev_pci.h>
> #include <rte_io.h>
> #include <rte_malloc.h>
> @@ -14,6 +15,35 @@
> #include "otx2_ethdev.h"
> #include "otx2_ethdev_sec.h"
>
> +uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> +int rte_pmd_octeontx2_timestamp_dynfield_offset = -1;
> +
> +static int
> +otx2_rx_timestamp_setup(uint16_t flags)
> +{
> + int timestamp_rx_dynflag_offset;
> +
> + if ((flags & NIX_RX_OFFLOAD_TSTAMP_F) == 0)
> + return 0;
> +
> + rte_pmd_octeontx2_timestamp_dynfield_offset = rte_mbuf_dynfield_lookup(
> + RTE_MBUF_DYNFIELD_TIMESTAMP_NAME, NULL);
> + if (rte_pmd_octeontx2_timestamp_dynfield_offset < 0) {
> + otx2_err("Failed to lookup timestamp field");
> + return -rte_errno;
> + }
> + timestamp_rx_dynflag_offset = rte_mbuf_dynflag_lookup(
> + RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME, NULL);
> + if (timestamp_rx_dynflag_offset < 0) {
> + otx2_err("Failed to lookup Rx timestamp flag");
> + return -rte_errno;
> + }
> + rte_pmd_octeontx2_timestamp_rx_dynflag =
> + RTE_BIT64(timestamp_rx_dynflag_offset);
> +
> + return 0;
> +}
> +
> static inline uint64_t
> nix_get_rx_offload_capa(struct otx2_eth_dev *dev)
> {
> @@ -1874,6 +1904,9 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
> dev->tx_offload_flags |= nix_tx_offload_flags(eth_dev);
> dev->rss_info.rss_grps = NIX_RSS_GRPS;
>
> + if (otx2_rx_timestamp_setup(dev->rx_offload_flags) != 0)
> + goto fail_offloads;
> +
> nb_rxq = RTE_MAX(data->nb_rx_queues, 1);
> nb_txq = RTE_MAX(data->nb_tx_queues, 1);
>
> diff --git a/drivers/net/octeontx2/otx2_rx.h b/drivers/net/octeontx2/otx2_rx.h
> index 61a5c436dd..6981edce82 100644
> --- a/drivers/net/octeontx2/otx2_rx.h
> +++ b/drivers/net/octeontx2/otx2_rx.h
> @@ -63,6 +63,18 @@ union mbuf_initializer {
> uint64_t value;
> };
>
> +/* variables are exported because this file is included in other drivers */
> +extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
> +
> +static inline rte_mbuf_timestamp_t *
> +otx2_timestamp_dynfield(struct rte_mbuf *mbuf)
> +{
> + return RTE_MBUF_DYNFIELD(mbuf,
> + rte_pmd_octeontx2_timestamp_dynfield_offset,
> + rte_mbuf_timestamp_t *);
> +}
> +
May be ethdev should provide the inline function?
29/10/2020 12:02, Andrew Rybchenko:
> On 10/29/20 12:27 PM, Thomas Monjalon wrote:
> > +/* variables are exported because this file is included in other drivers */
> > +extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> > +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
> > +
> > +static inline rte_mbuf_timestamp_t *
> > +otx2_timestamp_dynfield(struct rte_mbuf *mbuf)
> > +{
> > + return RTE_MBUF_DYNFIELD(mbuf,
> > + rte_pmd_octeontx2_timestamp_dynfield_offset,
> > + rte_mbuf_timestamp_t *);
> > +}
> > +
>
> May be ethdev should provide the inline function?
It would mean exporting the offsets.
And actually I think this field should not be restricted to ethdev.
On 10/29/20 2:34 PM, Thomas Monjalon wrote:
> 29/10/2020 12:02, Andrew Rybchenko:
>> On 10/29/20 12:27 PM, Thomas Monjalon wrote:
>>> +/* variables are exported because this file is included in other drivers */
>>> +extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
>>> +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
>>> +
>>> +static inline rte_mbuf_timestamp_t *
>>> +otx2_timestamp_dynfield(struct rte_mbuf *mbuf)
>>> +{
>>> + return RTE_MBUF_DYNFIELD(mbuf,
>>> + rte_pmd_octeontx2_timestamp_dynfield_offset,
>>> + rte_mbuf_timestamp_t *);
>>> +}
>>> +
>>
>> May be ethdev should provide the inline function?
>
> It would mean exporting the offsets.
>
> And actually I think this field should not be restricted to ethdev.
>
I see OK.
Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
Offset might be located in some memory sharing the cacheline with some other variables.
If these variables are writable and are being updated frequently - we might get the cache contention.
I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
attributes for these ones. Hence, exporting is OK, but practical usage in datapath is questionable.
With best regards, Slava
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, October 29, 2020 13:02
> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> Cc: ferruh.yigit@intel.com; david.marchand@redhat.com;
> bruce.richardson@intel.com; olivier.matz@6wind.com; jerinj@marvell.com;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> Subject: Re: [PATCH 10/15] net/octeontx2: switch timestamp to dynamic mbuf
> field
>
> On 10/29/20 12:27 PM, Thomas Monjalon wrote:
> > The mbuf timestamp is moved to a dynamic field in order to allow
> > removal of the deprecated static field.
> > The related mbuf flag is also replaced.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > drivers/net/octeontx2/otx2_ethdev.c | 33
> +++++++++++++++++++++++++++++
> > drivers/net/octeontx2/otx2_rx.h | 19 ++++++++++++++---
> > drivers/net/octeontx2/version.map | 7 ++++++
> > 3 files changed, 56 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/octeontx2/otx2_ethdev.c
> > b/drivers/net/octeontx2/otx2_ethdev.c
> > index cfb733a4b5..ad95219438 100644
> > --- a/drivers/net/octeontx2/otx2_ethdev.c
> > +++ b/drivers/net/octeontx2/otx2_ethdev.c
> > @@ -4,6 +4,7 @@
> >
> > #include <inttypes.h>
> >
> > +#include <rte_bitops.h>
> > #include <rte_ethdev_pci.h>
> > #include <rte_io.h>
> > #include <rte_malloc.h>
> > @@ -14,6 +15,35 @@
> > #include "otx2_ethdev.h"
> > #include "otx2_ethdev_sec.h"
> >
> > +uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> > +int rte_pmd_octeontx2_timestamp_dynfield_offset = -1;
> > +
> > +static int
> > +otx2_rx_timestamp_setup(uint16_t flags) {
> > + int timestamp_rx_dynflag_offset;
> > +
> > + if ((flags & NIX_RX_OFFLOAD_TSTAMP_F) == 0)
> > + return 0;
> > +
> > + rte_pmd_octeontx2_timestamp_dynfield_offset =
> rte_mbuf_dynfield_lookup(
> > + RTE_MBUF_DYNFIELD_TIMESTAMP_NAME, NULL);
> > + if (rte_pmd_octeontx2_timestamp_dynfield_offset < 0) {
> > + otx2_err("Failed to lookup timestamp field");
> > + return -rte_errno;
> > + }
> > + timestamp_rx_dynflag_offset = rte_mbuf_dynflag_lookup(
> > + RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME, NULL);
> > + if (timestamp_rx_dynflag_offset < 0) {
> > + otx2_err("Failed to lookup Rx timestamp flag");
> > + return -rte_errno;
> > + }
> > + rte_pmd_octeontx2_timestamp_rx_dynflag =
> > + RTE_BIT64(timestamp_rx_dynflag_offset);
> > +
> > + return 0;
> > +}
> > +
> > static inline uint64_t
> > nix_get_rx_offload_capa(struct otx2_eth_dev *dev) { @@ -1874,6
> > +1904,9 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
> > dev->tx_offload_flags |= nix_tx_offload_flags(eth_dev);
> > dev->rss_info.rss_grps = NIX_RSS_GRPS;
> >
> > + if (otx2_rx_timestamp_setup(dev->rx_offload_flags) != 0)
> > + goto fail_offloads;
> > +
> > nb_rxq = RTE_MAX(data->nb_rx_queues, 1);
> > nb_txq = RTE_MAX(data->nb_tx_queues, 1);
> >
> > diff --git a/drivers/net/octeontx2/otx2_rx.h
> > b/drivers/net/octeontx2/otx2_rx.h index 61a5c436dd..6981edce82 100644
> > --- a/drivers/net/octeontx2/otx2_rx.h
> > +++ b/drivers/net/octeontx2/otx2_rx.h
> > @@ -63,6 +63,18 @@ union mbuf_initializer {
> > uint64_t value;
> > };
> >
> > +/* variables are exported because this file is included in other
> > +drivers */ extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> > +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
> > +
> > +static inline rte_mbuf_timestamp_t *
> > +otx2_timestamp_dynfield(struct rte_mbuf *mbuf) {
> > + return RTE_MBUF_DYNFIELD(mbuf,
> > + rte_pmd_octeontx2_timestamp_dynfield_offset,
> > + rte_mbuf_timestamp_t *);
> > +}
> > +
>
> May be ethdev should provide the inline function?
Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
Offset might be located in some memory sharing the cacheline with some other variables.
If these variables are writable and are being updated frequently - we might get the cache contention.
I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
attributes for these ones. Hence, exporting/inline function is possible,
but practical usage, say, in datapath, is questionable.
With best regards, Slava
On Thu, Oct 29, 2020 at 5:22 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
>
> Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
I agree with Slava. The offset value should be stored in the PMD structure.
IMO, We can have an ethdev API to get the offset and store it in PMD's
fastpath structures in the slow path
to use in fastpath.
> Offset might be located in some memory sharing the cacheline with some other variables.
> If these variables are writable and are being updated frequently - we might get the cache contention.
> I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
> attributes for these ones. Hence, exporting is OK, but practical usage in datapath is questionable.
>
> With best regards, Slava
>
> > -----Original Message-----
> > From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Sent: Thursday, October 29, 2020 13:02
> > To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > Cc: ferruh.yigit@intel.com; david.marchand@redhat.com;
> > bruce.richardson@intel.com; olivier.matz@6wind.com; jerinj@marvell.com;
> > Slava Ovsiienko <viacheslavo@nvidia.com>; Nithin Dabilpuram
> > <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
> > Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> > Subject: Re: [PATCH 10/15] net/octeontx2: switch timestamp to dynamic mbuf
> > field
> >
> > On 10/29/20 12:27 PM, Thomas Monjalon wrote:
> > > The mbuf timestamp is moved to a dynamic field in order to allow
> > > removal of the deprecated static field.
> > > The related mbuf flag is also replaced.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > > drivers/net/octeontx2/otx2_ethdev.c | 33
> > +++++++++++++++++++++++++++++
> > > drivers/net/octeontx2/otx2_rx.h | 19 ++++++++++++++---
> > > drivers/net/octeontx2/version.map | 7 ++++++
> > > 3 files changed, 56 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/octeontx2/otx2_ethdev.c
> > > b/drivers/net/octeontx2/otx2_ethdev.c
> > > index cfb733a4b5..ad95219438 100644
> > > --- a/drivers/net/octeontx2/otx2_ethdev.c
> > > +++ b/drivers/net/octeontx2/otx2_ethdev.c
> > > @@ -4,6 +4,7 @@
> > >
> > > #include <inttypes.h>
> > >
> > > +#include <rte_bitops.h>
> > > #include <rte_ethdev_pci.h>
> > > #include <rte_io.h>
> > > #include <rte_malloc.h>
> > > @@ -14,6 +15,35 @@
> > > #include "otx2_ethdev.h"
> > > #include "otx2_ethdev_sec.h"
> > >
> > > +uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> > > +int rte_pmd_octeontx2_timestamp_dynfield_offset = -1;
> > > +
> > > +static int
> > > +otx2_rx_timestamp_setup(uint16_t flags) {
> > > + int timestamp_rx_dynflag_offset;
> > > +
> > > + if ((flags & NIX_RX_OFFLOAD_TSTAMP_F) == 0)
> > > + return 0;
> > > +
> > > + rte_pmd_octeontx2_timestamp_dynfield_offset =
> > rte_mbuf_dynfield_lookup(
> > > + RTE_MBUF_DYNFIELD_TIMESTAMP_NAME, NULL);
> > > + if (rte_pmd_octeontx2_timestamp_dynfield_offset < 0) {
> > > + otx2_err("Failed to lookup timestamp field");
> > > + return -rte_errno;
> > > + }
> > > + timestamp_rx_dynflag_offset = rte_mbuf_dynflag_lookup(
> > > + RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME, NULL);
> > > + if (timestamp_rx_dynflag_offset < 0) {
> > > + otx2_err("Failed to lookup Rx timestamp flag");
> > > + return -rte_errno;
> > > + }
> > > + rte_pmd_octeontx2_timestamp_rx_dynflag =
> > > + RTE_BIT64(timestamp_rx_dynflag_offset);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static inline uint64_t
> > > nix_get_rx_offload_capa(struct otx2_eth_dev *dev) { @@ -1874,6
> > > +1904,9 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
> > > dev->tx_offload_flags |= nix_tx_offload_flags(eth_dev);
> > > dev->rss_info.rss_grps = NIX_RSS_GRPS;
> > >
> > > + if (otx2_rx_timestamp_setup(dev->rx_offload_flags) != 0)
> > > + goto fail_offloads;
> > > +
> > > nb_rxq = RTE_MAX(data->nb_rx_queues, 1);
> > > nb_txq = RTE_MAX(data->nb_tx_queues, 1);
> > >
> > > diff --git a/drivers/net/octeontx2/otx2_rx.h
> > > b/drivers/net/octeontx2/otx2_rx.h index 61a5c436dd..6981edce82 100644
> > > --- a/drivers/net/octeontx2/otx2_rx.h
> > > +++ b/drivers/net/octeontx2/otx2_rx.h
> > > @@ -63,6 +63,18 @@ union mbuf_initializer {
> > > uint64_t value;
> > > };
> > >
> > > +/* variables are exported because this file is included in other
> > > +drivers */ extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
> > > +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
> > > +
> > > +static inline rte_mbuf_timestamp_t *
> > > +otx2_timestamp_dynfield(struct rte_mbuf *mbuf) {
> > > + return RTE_MBUF_DYNFIELD(mbuf,
> > > + rte_pmd_octeontx2_timestamp_dynfield_offset,
> > > + rte_mbuf_timestamp_t *);
> > > +}
> > > +
> >
> > May be ethdev should provide the inline function?
> Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
> Offset might be located in some memory sharing the cacheline with some other variables.
> If these variables are writable and are being updated frequently - we might get the cache contention.
> I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
> attributes for these ones. Hence, exporting/inline function is possible,
> but practical usage, say, in datapath, is questionable.
>
> With best regards, Slava
>
30/10/2020 13:41, Jerin Jacob:
> On Thu, Oct 29, 2020 at 5:22 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
> > From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >
> > Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
>
> I agree with Slava. The offset value should be stored in the PMD structure.
> IMO, We can have an ethdev API to get the offset and store it in PMD's
> fastpath structures in the slow path
> to use in fastpath.
>
> > Offset might be located in some memory sharing the cacheline with some other variables.
> > If these variables are writable and are being updated frequently - we might get the cache contention.
> > I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
> > attributes for these ones. Hence, exporting is OK, but practical usage in datapath is questionable.
Yes this is a major design point:
the field offsets are preferably stored in a hot cache line
which depends on the driver, library or application context.
On 10/30/20 3:41 PM, Jerin Jacob wrote:
> On Thu, Oct 29, 2020 at 5:22 PM Slava Ovsiienko <viacheslavo@nvidia.com> wrote:
>> Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
> I agree with Slava. The offset value should be stored in the PMD structure.
> IMO, We can have an ethdev API to get the offset and store it in PMD's
> fastpath structures in the slow path
> to use in fastpath.
I don't mind. My main goal is to raise the topic and, may be, add a bit
more comments in the code to help the future maintainers to
understand it. It is not trivial topic and any help to code readers in
the comments would be very useful.
>> Offset might be located in some memory sharing the cacheline with some other variables.
>> If these variables are writable and are being updated frequently - we might get the cache contention.
>> I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
>> attributes for these ones. Hence, exporting is OK, but practical usage in datapath is questionable.
>>
>> With best regards, Slava
>>
>>> -----Original Message-----
>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> Sent: Thursday, October 29, 2020 13:02
>>> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
>>> Cc: ferruh.yigit@intel.com; david.marchand@redhat.com;
>>> bruce.richardson@intel.com; olivier.matz@6wind.com; jerinj@marvell.com;
>>> Slava Ovsiienko <viacheslavo@nvidia.com>; Nithin Dabilpuram
>>> <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
>>> Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
>>> Subject: Re: [PATCH 10/15] net/octeontx2: switch timestamp to dynamic mbuf
>>> field
>>>
>>> On 10/29/20 12:27 PM, Thomas Monjalon wrote:
>>>> The mbuf timestamp is moved to a dynamic field in order to allow
>>>> removal of the deprecated static field.
>>>> The related mbuf flag is also replaced.
>>>>
>>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>>> ---
>>>> drivers/net/octeontx2/otx2_ethdev.c | 33
>>> +++++++++++++++++++++++++++++
>>>> drivers/net/octeontx2/otx2_rx.h | 19 ++++++++++++++---
>>>> drivers/net/octeontx2/version.map | 7 ++++++
>>>> 3 files changed, 56 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/octeontx2/otx2_ethdev.c
>>>> b/drivers/net/octeontx2/otx2_ethdev.c
>>>> index cfb733a4b5..ad95219438 100644
>>>> --- a/drivers/net/octeontx2/otx2_ethdev.c
>>>> +++ b/drivers/net/octeontx2/otx2_ethdev.c
>>>> @@ -4,6 +4,7 @@
>>>>
>>>> #include <inttypes.h>
>>>>
>>>> +#include <rte_bitops.h>
>>>> #include <rte_ethdev_pci.h>
>>>> #include <rte_io.h>
>>>> #include <rte_malloc.h>
>>>> @@ -14,6 +15,35 @@
>>>> #include "otx2_ethdev.h"
>>>> #include "otx2_ethdev_sec.h"
>>>>
>>>> +uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
>>>> +int rte_pmd_octeontx2_timestamp_dynfield_offset = -1;
>>>> +
>>>> +static int
>>>> +otx2_rx_timestamp_setup(uint16_t flags) {
>>>> + int timestamp_rx_dynflag_offset;
>>>> +
>>>> + if ((flags & NIX_RX_OFFLOAD_TSTAMP_F) == 0)
>>>> + return 0;
>>>> +
>>>> + rte_pmd_octeontx2_timestamp_dynfield_offset =
>>> rte_mbuf_dynfield_lookup(
>>>> + RTE_MBUF_DYNFIELD_TIMESTAMP_NAME, NULL);
>>>> + if (rte_pmd_octeontx2_timestamp_dynfield_offset < 0) {
>>>> + otx2_err("Failed to lookup timestamp field");
>>>> + return -rte_errno;
>>>> + }
>>>> + timestamp_rx_dynflag_offset = rte_mbuf_dynflag_lookup(
>>>> + RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME, NULL);
>>>> + if (timestamp_rx_dynflag_offset < 0) {
>>>> + otx2_err("Failed to lookup Rx timestamp flag");
>>>> + return -rte_errno;
>>>> + }
>>>> + rte_pmd_octeontx2_timestamp_rx_dynflag =
>>>> + RTE_BIT64(timestamp_rx_dynflag_offset);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static inline uint64_t
>>>> nix_get_rx_offload_capa(struct otx2_eth_dev *dev) { @@ -1874,6
>>>> +1904,9 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
>>>> dev->tx_offload_flags |= nix_tx_offload_flags(eth_dev);
>>>> dev->rss_info.rss_grps = NIX_RSS_GRPS;
>>>>
>>>> + if (otx2_rx_timestamp_setup(dev->rx_offload_flags) != 0)
>>>> + goto fail_offloads;
>>>> +
>>>> nb_rxq = RTE_MAX(data->nb_rx_queues, 1);
>>>> nb_txq = RTE_MAX(data->nb_tx_queues, 1);
>>>>
>>>> diff --git a/drivers/net/octeontx2/otx2_rx.h
>>>> b/drivers/net/octeontx2/otx2_rx.h index 61a5c436dd..6981edce82 100644
>>>> --- a/drivers/net/octeontx2/otx2_rx.h
>>>> +++ b/drivers/net/octeontx2/otx2_rx.h
>>>> @@ -63,6 +63,18 @@ union mbuf_initializer {
>>>> uint64_t value;
>>>> };
>>>>
>>>> +/* variables are exported because this file is included in other
>>>> +drivers */ extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
>>>> +extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
>>>> +
>>>> +static inline rte_mbuf_timestamp_t *
>>>> +otx2_timestamp_dynfield(struct rte_mbuf *mbuf) {
>>>> + return RTE_MBUF_DYNFIELD(mbuf,
>>>> + rte_pmd_octeontx2_timestamp_dynfield_offset,
>>>> + rte_mbuf_timestamp_t *);
>>>> +}
>>>> +
>>> May be ethdev should provide the inline function?
>> Just five cents - exporting the offset (making it global) might have side effect impacting the performance.
>> Offset might be located in some memory sharing the cacheline with some other variables.
>> If these variables are writable and are being updated frequently - we might get the cache contention.
>> I'd prefer to keep all dynamic offsets In the PMD and entirely control memory allocation
>> attributes for these ones. Hence, exporting/inline function is possible,
>> but practical usage, say, in datapath, is questionable.
>>
>> With best regards, Slava
>>
@@ -4,6 +4,7 @@
#include <inttypes.h>
+#include <rte_bitops.h>
#include <rte_ethdev_pci.h>
#include <rte_io.h>
#include <rte_malloc.h>
@@ -14,6 +15,35 @@
#include "otx2_ethdev.h"
#include "otx2_ethdev_sec.h"
+uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
+int rte_pmd_octeontx2_timestamp_dynfield_offset = -1;
+
+static int
+otx2_rx_timestamp_setup(uint16_t flags)
+{
+ int timestamp_rx_dynflag_offset;
+
+ if ((flags & NIX_RX_OFFLOAD_TSTAMP_F) == 0)
+ return 0;
+
+ rte_pmd_octeontx2_timestamp_dynfield_offset = rte_mbuf_dynfield_lookup(
+ RTE_MBUF_DYNFIELD_TIMESTAMP_NAME, NULL);
+ if (rte_pmd_octeontx2_timestamp_dynfield_offset < 0) {
+ otx2_err("Failed to lookup timestamp field");
+ return -rte_errno;
+ }
+ timestamp_rx_dynflag_offset = rte_mbuf_dynflag_lookup(
+ RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME, NULL);
+ if (timestamp_rx_dynflag_offset < 0) {
+ otx2_err("Failed to lookup Rx timestamp flag");
+ return -rte_errno;
+ }
+ rte_pmd_octeontx2_timestamp_rx_dynflag =
+ RTE_BIT64(timestamp_rx_dynflag_offset);
+
+ return 0;
+}
+
static inline uint64_t
nix_get_rx_offload_capa(struct otx2_eth_dev *dev)
{
@@ -1874,6 +1904,9 @@ otx2_nix_configure(struct rte_eth_dev *eth_dev)
dev->tx_offload_flags |= nix_tx_offload_flags(eth_dev);
dev->rss_info.rss_grps = NIX_RSS_GRPS;
+ if (otx2_rx_timestamp_setup(dev->rx_offload_flags) != 0)
+ goto fail_offloads;
+
nb_rxq = RTE_MAX(data->nb_rx_queues, 1);
nb_txq = RTE_MAX(data->nb_tx_queues, 1);
@@ -63,6 +63,18 @@ union mbuf_initializer {
uint64_t value;
};
+/* variables are exported because this file is included in other drivers */
+extern uint64_t rte_pmd_octeontx2_timestamp_rx_dynflag;
+extern int rte_pmd_octeontx2_timestamp_dynfield_offset;
+
+static inline rte_mbuf_timestamp_t *
+otx2_timestamp_dynfield(struct rte_mbuf *mbuf)
+{
+ return RTE_MBUF_DYNFIELD(mbuf,
+ rte_pmd_octeontx2_timestamp_dynfield_offset,
+ rte_mbuf_timestamp_t *);
+}
+
static __rte_always_inline void
otx2_nix_mbuf_to_tstamp(struct rte_mbuf *mbuf,
struct otx2_timesync_info *tstamp, const uint16_t flag,
@@ -77,15 +89,16 @@ otx2_nix_mbuf_to_tstamp(struct rte_mbuf *mbuf,
/* Reading the rx timestamp inserted by CGX, viz at
* starting of the packet data.
*/
- mbuf->timestamp = rte_be_to_cpu_64(*tstamp_ptr);
+ *otx2_timestamp_dynfield(mbuf) = rte_be_to_cpu_64(*tstamp_ptr);
/* PKT_RX_IEEE1588_TMST flag needs to be set only in case
* PTP packets are received.
*/
if (mbuf->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC) {
- tstamp->rx_tstamp = mbuf->timestamp;
+ tstamp->rx_tstamp = *otx2_timestamp_dynfield(mbuf);
tstamp->rx_ready = 1;
mbuf->ol_flags |= PKT_RX_IEEE1588_PTP |
- PKT_RX_IEEE1588_TMST | PKT_RX_TIMESTAMP;
+ PKT_RX_IEEE1588_TMST |
+ rte_pmd_octeontx2_timestamp_rx_dynflag;
}
}
}
@@ -1,3 +1,10 @@
DPDK_21 {
local: *;
};
+
+INTERNAL {
+ global:
+
+ rte_pmd_octeontx2_timestamp_dynfield_offset;
+ rte_pmd_octeontx2_timestamp_rx_dynflag;
+};