Re: [dpdk-dev] [RFC PATCH 13/14] mbuf: cleanup + added in additional mbuf fields.

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Bruce,

On 08/11/2014 10:44 PM, Bruce Richardson wrote:
> Cleanups:
> * use typedefs for markers within mbuf struct
> * split up vlan_macip field as the l2/l3 lengths are for TX so go on the
>    second cache line.
> * created a tx_ol field in second cache line for data used for tx
>    offloads
> * rename the hash field to the filter field as it contains more than
>    just a hash value.
>
> Added in the extra mbuf fields needed:
> * fdir flex bytes for i40e driver, i.e. extra 32-bits for filters
> * field to be used for a sequence number, extra 32-bit field
> * field for a second vlan tag, extra 16-bits, using space freed by
>    moving out the l2 l3 lengths.
> * userdata field for general application use.
> * added inner_l3 and l4 length fields to allow tunneling.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

I think it would be clearer if splitted in several patches. First
2 patches for:

- rename hash in filter
- rename vlan_macip in rte_tx_offloads (and change to 64 bits ?)

By the way, the modification of ol_flags in 64 bits probably
requires more modifications in driver and testpmd. See me initial
patch "mbuf: change ol_flags to 32 bits".

Why not directly adding the MARKER typedef in previous patches?
Also, I'm wondering if the markers really need to by typed. An empty
struct would do the job, just requiring the users to cast it into
the proper type.

About sequence, double vlan id, userdata: they are not used now.
I think we should add them one by one in separate patches, with
code really that really requires the added field to be present.
By the way, there is already a way to reserve a zone in mbuf for
the application at mbuf pool creation. What is the purpose of
the userdata field?

About fdir_i40e field, I'm not sure that we should have
driver-specific info in the generic mbuf structure. In addition,
the application would need to know which hash type is present
in the mbuf.

Each time we add a new field in the mbuf, I think we should ask
ourselves how the application will use it, especially if the
application manages several kind of ports (virtual and
physical for instance), which may not support all features.
For instance, who will fill the "sequence" field ? Is it the
driver? If it's application-specific

Regards,
Olivier