mbox series

[v6,00/15] remove mbuf userdata

Message ID 20201030174441.1076264-1-thomas@monjalon.net (mailing list archive)
Headers
Series remove mbuf userdata |

Message

Thomas Monjalon Oct. 30, 2020, 5:44 p.m. UTC
  The mbuf field userdata (aliased as udata64)
was announced to be removed for two reasons:
  - applications, libraries and drivers used the same field
    for different purposes, with a risk of usage conflict.
  - this field always used 8 bytes even if unneeded

Some dynamic fields are created when needed to replace
the big static userdata field.
As a consequence, 8 bytes can be re-allocated to dynamic fields.

This mbuf layout change is important to allow adding more features
(consuming more dynamic fields) during the next year.


v6:
- ARK field types different for Rx and Tx
- replace macros with inline functions

v5: sent "timestamp series" by mistake

v4 (thanks Jerin, Nithin and Ed):
- move rte_node dynfield offset to hotter context in cache
- move test-eventdev dynfield offset to hotter context in cache
- split ARK user data for Rx and Tx + more ARK fixes

v3 (thanks Olivier):
- use typedef for new field types
- fix node field type
- initialize offsets to -1
- add more inline functions
- inline rte_security_dynfield_is_registered
- add PMD-specific userdata field for ARK

v2 (thanks David & Andrew):
- fix some indentations
- return -rte_errno consistently
- make some type casts more precise
- define dynfield types in macros
- hide field description in rte_security
- do not lookup security dynfield in ipsec-secgw
- do not use the existing timestamp field for other purpose


Ed Czeck (1):
  net/ark: switch user data to dynamic mbuf fields

Nithin Dabilpuram (1):
  node: switch IPv4 metadata to dynamic mbuf field

Thomas Monjalon (13):
  examples: enclose DPDK includes with angle brackets
  kni: move header file from EAL
  mbuf: fix typo in dynamic field convention note
  security: switch metadata to dynamic mbuf field
  event/sw: switch test counter to dynamic mbuf field
  net/bnxt: switch CFA code to dynamic mbuf field
  net/vmxnet3: switch MSS hint to dynamic mbuf field
  test/distributor: switch sequence to dynamic mbuf field
  test/graph: switch user data to dynamic mbuf field
  app/eventdev: switch flow ID to dynamic mbuf field
  examples/bbdev: switch to dynamic mbuf field
  examples/rxtx_callbacks: switch TSC to dynamic field
  mbuf: remove userdata field

 app/test-eventdev/test_order_atq.c            |   4 +-
 app/test-eventdev/test_order_common.c         |  21 ++-
 app/test-eventdev/test_order_common.h         |  21 +++
 app/test-eventdev/test_order_queue.c          |   4 +-
 app/test/test_distributor.c                   |  32 ++++-
 app/test/test_graph.c                         |  99 ++++++++------
 doc/api/doxy-api-index.md                     |   1 +
 doc/api/doxy-api.conf.in                      |   1 +
 doc/guides/prog_guide/rte_security.rst        |   9 +-
 doc/guides/rel_notes/deprecation.rst          |   1 -
 doc/guides/rel_notes/release_20_11.rst        |   3 +
 doc/guides/sample_app_ug/rxtx_callbacks.rst   |   4 +-
 drivers/crypto/octeontx2/otx2_cryptodev_sec.c |   5 +-
 drivers/event/sw/sw_evdev_selftest.c          |  28 +++-
 drivers/net/ark/ark_ethdev.c                  |  39 ++++++
 drivers/net/ark/ark_ethdev_rx.c               |   3 +-
 drivers/net/ark/ark_ethdev_tx.c               |   3 +-
 drivers/net/ark/meson.build                   |   2 +
 drivers/net/ark/rte_pmd_ark.h                 | 125 ++++++++++++++++++
 drivers/net/ark/version.map                   |   7 +
 drivers/net/bnxt/bnxt_ethdev.c                |  19 +++
 drivers/net/bnxt/bnxt_rxr.c                   |   2 +-
 drivers/net/bnxt/bnxt_rxr.h                   |  10 ++
 drivers/net/bnxt/rte_pmd_bnxt.h               |   3 +
 drivers/net/ixgbe/ixgbe_ipsec.c               |   5 +-
 drivers/net/ixgbe/ixgbe_rxtx.c                |   6 +-
 drivers/net/octeontx2/otx2_ethdev.h           |   1 +
 drivers/net/octeontx2/otx2_ethdev_sec.c       |   5 +-
 drivers/net/octeontx2/otx2_ethdev_sec_tx.h    |   2 +-
 drivers/net/octeontx2/otx2_rx.h               |   2 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c          |  15 +++
 drivers/net/vmxnet3/vmxnet3_ethdev.h          |  11 ++
 drivers/net/vmxnet3/vmxnet3_rxtx.c            |   9 +-
 examples/bbdev_app/main.c                     |  72 ++++++----
 examples/bond/main.c                          |  11 +-
 examples/ipsec-secgw/ipsec-secgw.c            |   9 +-
 examples/ipsec-secgw/ipsec_worker.c           |  12 +-
 examples/rxtx_callbacks/main.c                |  24 +++-
 kernel/linux/kni/meson.build                  |   2 +-
 lib/librte_eal/linux/include/meson.build      |   1 -
 lib/librte_kni/meson.build                    |   2 +-
 .../include => librte_kni}/rte_kni_common.h   |   2 +-
 lib/librte_mbuf/rte_mbuf_core.h               |   8 +-
 lib/librte_mbuf/rte_mbuf_dyn.h                |   2 +-
 lib/librte_node/ip4_lookup.c                  |  40 ++++--
 lib/librte_node/ip4_lookup_neon.h             |  20 ++-
 lib/librte_node/ip4_lookup_sse.h              |  36 +++--
 lib/librte_node/ip4_rewrite.c                 |  49 +++++--
 lib/librte_node/node_private.h                |  13 +-
 lib/librte_security/rte_security.c            |  16 +++
 lib/librte_security/rte_security.h            |  42 ++++++
 lib/librte_security/rte_security_driver.h     |   3 +
 lib/librte_security/version.map               |   2 +
 53 files changed, 681 insertions(+), 187 deletions(-)
 create mode 100644 drivers/net/ark/rte_pmd_ark.h
 rename lib/{librte_eal/linux/include => librte_kni}/rte_kni_common.h (98%)
  

Comments

Thomas Monjalon Oct. 31, 2020, 3:07 p.m. UTC | #1
30/10/2020 18:44, Thomas Monjalon:
> The mbuf field userdata (aliased as udata64)
> was announced to be removed for two reasons:
>   - applications, libraries and drivers used the same field
>     for different purposes, with a risk of usage conflict.
>   - this field always used 8 bytes even if unneeded
> 
> Some dynamic fields are created when needed to replace
> the big static userdata field.
> As a consequence, 8 bytes can be re-allocated to dynamic fields.
> 
> This mbuf layout change is important to allow adding more features
> (consuming more dynamic fields) during the next year.

Applied
  
Ferruh Yigit Oct. 31, 2020, 11:36 p.m. UTC | #2
On 10/31/2020 3:07 PM, Thomas Monjalon wrote:
> 30/10/2020 18:44, Thomas Monjalon:
>> The mbuf field userdata (aliased as udata64)
>> was announced to be removed for two reasons:
>>    - applications, libraries and drivers used the same field
>>      for different purposes, with a risk of usage conflict.
>>    - this field always used 8 bytes even if unneeded
>>
>> Some dynamic fields are created when needed to replace
>> the big static userdata field.
>> As a consequence, 8 bytes can be re-allocated to dynamic fields.
>>
>> This mbuf layout change is important to allow adding more features
>> (consuming more dynamic fields) during the next year.
> 
> Applied
> 

The new txgbe driver in the next-net is also using ‘udata64’, that also needs to 
be updated. cc'ed txgbe maintainer.
  
Thomas Monjalon Nov. 1, 2020, 9:15 a.m. UTC | #3
01/11/2020 00:36, Ferruh Yigit:
> On 10/31/2020 3:07 PM, Thomas Monjalon wrote:
> > 30/10/2020 18:44, Thomas Monjalon:
> >> The mbuf field userdata (aliased as udata64)
> >> was announced to be removed for two reasons:
> >>    - applications, libraries and drivers used the same field
> >>      for different purposes, with a risk of usage conflict.
> >>    - this field always used 8 bytes even if unneeded
> >>
> >> Some dynamic fields are created when needed to replace
> >> the big static userdata field.
> >> As a consequence, 8 bytes can be re-allocated to dynamic fields.
> >>
> >> This mbuf layout change is important to allow adding more features
> >> (consuming more dynamic fields) during the next year.
> > 
> > Applied
> 
> The new txgbe driver in the next-net is also using ‘udata64’, that also needs to 
> be updated. cc'ed txgbe maintainer.

That's a pity it did not take into account the deprecation notice.
What kind of hack is it used for?
Can it be simply removed to allow quick merging of the PMD?
  
David Marchand Nov. 1, 2020, 10:26 a.m. UTC | #4
On Sun, Nov 1, 2020 at 10:15 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > The new txgbe driver in the next-net is also using ‘udata64’, that also needs to
> > be updated. cc'ed txgbe maintainer.
>
> That's a pity it did not take into account the deprecation notice.
> What kind of hack is it used for?
> Can it be simply removed to allow quick merging of the PMD?

+1 for removing.

It seems to be a provision for future features, as this field is
simply passed to an internal function that does not use it.

$ git grep -C 2 udata drivers/net/txgbe/
drivers/net/txgbe/txgbe_rxtx.c-
drivers/net/txgbe/txgbe_rxtx.c-
txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
drivers/net/txgbe/txgbe_rxtx.c:
tx_offload, &tx_pkt->udata64);
drivers/net/txgbe/txgbe_rxtx.c-
drivers/net/txgbe/txgbe_rxtx.c-                         txe->last_id = tx_last;

$ git grep -C 2 txgbe_set_xmit_ctx drivers/net/txgbe/
drivers/net/txgbe/txgbe_rxtx.c-
drivers/net/txgbe/txgbe_rxtx.c-static inline void
drivers/net/txgbe/txgbe_rxtx.c:txgbe_set_xmit_ctx(struct txgbe_tx_queue *txq,
drivers/net/txgbe/txgbe_rxtx.c-         volatile struct
txgbe_tx_ctx_desc *ctx_txd,
drivers/net/txgbe/txgbe_rxtx.c-         uint64_t ol_flags, union
txgbe_tx_offload tx_offload,
--
drivers/net/txgbe/txgbe_rxtx.c-                         }
drivers/net/txgbe/txgbe_rxtx.c-
drivers/net/txgbe/txgbe_rxtx.c:
txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
drivers/net/txgbe/txgbe_rxtx.c-
tx_offload, &tx_pkt->udata64);
drivers/net/txgbe/txgbe_rxtx.c-

$ git grep -w mdata drivers/net/txgbe/
drivers/net/txgbe/txgbe_rxtx.c:         __rte_unused uint64_t *mdata)
  
Jiawen Wu Nov. 2, 2020, 9:11 a.m. UTC | #5
On Sunday, November 1, 2020 6:26 PM, David Marchand wrote:
> On Sun, Nov 1, 2020 at 10:15 AM Thomas Monjalon <thomas@monjalon.net>
> wrote:
> > > The new txgbe driver in the next-net is also using ‘udata64’, that
> > > also needs to be updated. cc'ed txgbe maintainer.
> >
> > That's a pity it did not take into account the deprecation notice.
> > What kind of hack is it used for?
> > Can it be simply removed to allow quick merging of the PMD?
> 
> +1 for removing.
> 
> It seems to be a provision for future features, as this field is simply passed to
> an internal function that does not use it.
> 
> $ git grep -C 2 udata drivers/net/txgbe/
> drivers/net/txgbe/txgbe_rxtx.c-
> drivers/net/txgbe/txgbe_rxtx.c-
> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
> drivers/net/txgbe/txgbe_rxtx.c:
> tx_offload, &tx_pkt->udata64);
> drivers/net/txgbe/txgbe_rxtx.c-
> drivers/net/txgbe/txgbe_rxtx.c-                         txe->last_id =
> tx_last;
> 
> $ git grep -C 2 txgbe_set_xmit_ctx drivers/net/txgbe/
> drivers/net/txgbe/txgbe_rxtx.c-
> drivers/net/txgbe/txgbe_rxtx.c-static inline void
> drivers/net/txgbe/txgbe_rxtx.c:txgbe_set_xmit_ctx(struct txgbe_tx_queue
> *txq,
> drivers/net/txgbe/txgbe_rxtx.c-         volatile struct
> txgbe_tx_ctx_desc *ctx_txd,
> drivers/net/txgbe/txgbe_rxtx.c-         uint64_t ol_flags, union
> txgbe_tx_offload tx_offload,
> --
> drivers/net/txgbe/txgbe_rxtx.c-                         }
> drivers/net/txgbe/txgbe_rxtx.c-
> drivers/net/txgbe/txgbe_rxtx.c:
> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
> drivers/net/txgbe/txgbe_rxtx.c-
> tx_offload, &tx_pkt->udata64);
> drivers/net/txgbe/txgbe_rxtx.c-
> 
> $ git grep -w mdata drivers/net/txgbe/
> drivers/net/txgbe/txgbe_rxtx.c:         __rte_unused uint64_t *mdata)
> 
> 
> --
> David Marchand

Thanks for review.
It can be just simply removed in txgbe driver.
  
Ferruh Yigit Nov. 2, 2020, 11:08 a.m. UTC | #6
On 11/2/2020 9:11 AM, Jiawen Wu wrote:
> On Sunday, November 1, 2020 6:26 PM, David Marchand wrote:
>> On Sun, Nov 1, 2020 at 10:15 AM Thomas Monjalon <thomas@monjalon.net>
>> wrote:
>>>> The new txgbe driver in the next-net is also using ‘udata64’, that
>>>> also needs to be updated. cc'ed txgbe maintainer.
>>>
>>> That's a pity it did not take into account the deprecation notice.
>>> What kind of hack is it used for?
>>> Can it be simply removed to allow quick merging of the PMD?
>>
>> +1 for removing.
>>
>> It seems to be a provision for future features, as this field is simply passed to
>> an internal function that does not use it.
>>
>> $ git grep -C 2 udata drivers/net/txgbe/
>> drivers/net/txgbe/txgbe_rxtx.c-
>> drivers/net/txgbe/txgbe_rxtx.c-
>> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
>> drivers/net/txgbe/txgbe_rxtx.c:
>> tx_offload, &tx_pkt->udata64);
>> drivers/net/txgbe/txgbe_rxtx.c-
>> drivers/net/txgbe/txgbe_rxtx.c-                         txe->last_id =
>> tx_last;
>>
>> $ git grep -C 2 txgbe_set_xmit_ctx drivers/net/txgbe/
>> drivers/net/txgbe/txgbe_rxtx.c-
>> drivers/net/txgbe/txgbe_rxtx.c-static inline void
>> drivers/net/txgbe/txgbe_rxtx.c:txgbe_set_xmit_ctx(struct txgbe_tx_queue
>> *txq,
>> drivers/net/txgbe/txgbe_rxtx.c-         volatile struct
>> txgbe_tx_ctx_desc *ctx_txd,
>> drivers/net/txgbe/txgbe_rxtx.c-         uint64_t ol_flags, union
>> txgbe_tx_offload tx_offload,
>> --
>> drivers/net/txgbe/txgbe_rxtx.c-                         }
>> drivers/net/txgbe/txgbe_rxtx.c-
>> drivers/net/txgbe/txgbe_rxtx.c:
>> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
>> drivers/net/txgbe/txgbe_rxtx.c-
>> tx_offload, &tx_pkt->udata64);
>> drivers/net/txgbe/txgbe_rxtx.c-
>>
>> $ git grep -w mdata drivers/net/txgbe/
>> drivers/net/txgbe/txgbe_rxtx.c:         __rte_unused uint64_t *mdata)
>>
>>
>> --
>> David Marchand
> 
> Thanks for review.
> It can be just simply removed in txgbe driver.
> 

OK, I will remove it in the next-net.
  
Ferruh Yigit Nov. 2, 2020, 11:58 a.m. UTC | #7
On 11/2/2020 11:08 AM, Ferruh Yigit wrote:
> On 11/2/2020 9:11 AM, Jiawen Wu wrote:
>> On Sunday, November 1, 2020 6:26 PM, David Marchand wrote:
>>> On Sun, Nov 1, 2020 at 10:15 AM Thomas Monjalon <thomas@monjalon.net>
>>> wrote:
>>>>> The new txgbe driver in the next-net is also using ‘udata64’, that
>>>>> also needs to be updated. cc'ed txgbe maintainer.
>>>>
>>>> That's a pity it did not take into account the deprecation notice.
>>>> What kind of hack is it used for?
>>>> Can it be simply removed to allow quick merging of the PMD?
>>>
>>> +1 for removing.
>>>
>>> It seems to be a provision for future features, as this field is simply 
>>> passed to
>>> an internal function that does not use it.
>>>
>>> $ git grep -C 2 udata drivers/net/txgbe/
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
>>> drivers/net/txgbe/txgbe_rxtx.c:
>>> tx_offload, &tx_pkt->udata64);
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> drivers/net/txgbe/txgbe_rxtx.c-                         txe->last_id =
>>> tx_last;
>>>
>>> $ git grep -C 2 txgbe_set_xmit_ctx drivers/net/txgbe/
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> drivers/net/txgbe/txgbe_rxtx.c-static inline void
>>> drivers/net/txgbe/txgbe_rxtx.c:txgbe_set_xmit_ctx(struct txgbe_tx_queue
>>> *txq,
>>> drivers/net/txgbe/txgbe_rxtx.c-         volatile struct
>>> txgbe_tx_ctx_desc *ctx_txd,
>>> drivers/net/txgbe/txgbe_rxtx.c-         uint64_t ol_flags, union
>>> txgbe_tx_offload tx_offload,
>>> -- 
>>> drivers/net/txgbe/txgbe_rxtx.c-                         }
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> drivers/net/txgbe/txgbe_rxtx.c:
>>> txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>> tx_offload, &tx_pkt->udata64);
>>> drivers/net/txgbe/txgbe_rxtx.c-
>>>
>>> $ git grep -w mdata drivers/net/txgbe/
>>> drivers/net/txgbe/txgbe_rxtx.c:         __rte_unused uint64_t *mdata)
>>>
>>>
>>> -- 
>>> David Marchand
>>
>> Thanks for review.
>> It can be just simply removed in txgbe driver.
>>
> 
> OK, I will remove it in the next-net.
> 

Applied following:

  diff --git a/drivers/net/txgbe/txgbe_rxtx.c b/drivers/net/txgbe/txgbe_rxtx.c
  index b35b7de1cb..4d8b43423d 100644
  --- a/drivers/net/txgbe/txgbe_rxtx.c
  +++ b/drivers/net/txgbe/txgbe_rxtx.c
  @@ -281,8 +281,7 @@ txgbe_xmit_pkts_simple(void *tx_queue, struct rte_mbuf 
**tx_pkts,
   static inline void
   txgbe_set_xmit_ctx(struct txgbe_tx_queue *txq,
                  volatile struct txgbe_tx_ctx_desc *ctx_txd,
  -               uint64_t ol_flags, union txgbe_tx_offload tx_offload,
  -               __rte_unused uint64_t *mdata)
  +               uint64_t ol_flags, union txgbe_tx_offload tx_offload)
   {
          union txgbe_tx_offload tx_offload_mask;
          uint32_t type_tucmd_mlhl;
  @@ -861,7 +860,7 @@ txgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
                                  }

                                  txgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
  -                                       tx_offload, &tx_pkt->udata64);
  +                                       tx_offload);

                                  txe->last_id = tx_last;
                                  tx_id = txe->next_id;