On 4/17/2024 8:23 AM, Michael Baum wrote:
> The "rte_flow_item_geneve_opt" structure describes the GENEVE TLV option
> header according to RFC 8926 [1]:
>
> struct rte_flow_item_geneve_opt {
> rte_be16_t option_class;
> uint8_t option_type;
> uint8_t option_len;
> uint32_t *data;
> };
>
> The "option_len" field is used for two different purposes:
> 1. item field for matching with value/mask.
> 2. descriptor for data array size.
>
For the long run solution, we may consider adding geneve option header
to net/rte_geneve.h and make "struct rte_flow_item_geneve_opt" + data size ?
> Those two different purposes might limit each other. For example, when
> matching on length with full mask (0x1f), the data array in the mask
> structure might be taken as size 31 and read invalid memory.
>
> This problem appears in conversion API. In current implementation, the
> "rte_flow_conv" API copies the "rte_flow_item_geneve_opt" structure
> without taking care about data deep-copy. The attempt to solve this
> revealed the problem in determining the size of the mask data array. To
> resolve this issue, two solutions are suggested.
>
Are we having this problem only with geneve options because data size is
not fixed / defined for the header?
> Immediate Workaround:
> The data array size in the "mask" structure is determined by
> "option_len" field in the "spec" structure. This workaround can be
> integrated soon to avoid deep-copy missing.
>
This requires a geneve specific pointer in the item spec, which is not
really nice, although it is temporary solution. Perhaps we can skip
this, can you please check below comment.
> Long Run Solution:
> Add a new field into "rte_flow_item_geneve_opt" structure regardless to
> "option_len" field. This solution should wait to "24.11" version since
> it contains API change.
>
I was expecting the same, but CI seems passed ABI test case [1], it may
be because new field appended end of the struct.
Can you please double check, if ABI is not broken, we can go with this
solution directly?
[1]
https://mails.dpdk.org/archives/test-report/2024-April/643570.html
> When the API is changed, I'll take the opportunity to add documentation
> for this item in "rte_flow.rst" file and update the data type to
> "rte_be32_t".
>
If we can go with updating struct in this release, adding protocol
option struct in net library can wait v24.11 release.
So "rte_be32_t" type change in this struct won't be a thing.
> [1] https://datatracker.ietf.org/doc/html/rfc8926
>
> Michael Baum (2):
> ethdev: fix GENEVE option item conversion
> ethdev: add data size field to GENEVE option item
>
>
@Ori, Can you please help reviewing this patch?
At worst, it can be good to address the fix in this release.
