Re: [dpdk-dev] [PATCH v8 06/10] ipsec: add transmit segmentation offload support

["Nicolau, Radu" <radu.nicolau@intel.com>]

On 10/12/2021 5:25 PM, Ananyev, Konstantin wrote:
>>> +/* check if packet will exceed MSS and segmentation is required */
>>> +static inline int
>>> +esn_outb_nb_segments(struct rte_mbuf *m) {
>> DPDK codying style pls.
>>
>>> +	uint16_t segments = 1;
>>> +	uint16_t pkt_l3len = m->pkt_len - m->l2_len;
>>> +
>>> +	/* Only support segmentation for UDP/TCP flows */
>>> +	if (!(m->packet_type & (RTE_PTYPE_L4_UDP | RTE_PTYPE_L4_TCP)))
>> For ptypes it is not a bit flag, it should be something like:
>>
>> pt =  m->packet_type & RTE_PTYPE_L4_MASK;
>> if (pt == RTE_PTYPE_L4_UDP || pt == RTE_PTYPE_L4_TCP) {...}
>>
>> BTW, ptype is usually used for RX path.
>> If you expect user to setup it up on TX path - it has to be documented in formal API comments.
> Thinking a bit more about it:
> Do we really need to force user to set ptypes to use this feature?
> Might be something as simple as follows would work:
>
> 1. If user expects that he would need TSO for the ESP packet,
> he would simply set PKT_TX_TCP_SEG flag and usual  offload fields required
> (l2_len, l3_len, l4_len, tso_segsz).
> 2. In ipsec lib we'll check for PKT_TX_TCP_SEG - and if it is set we'll do extra processing
> (as your patch does - calc number of segments, fill ESP data in a bit different way,
> fill outer_l2_len, outer_l3_len etc.)
> 3. If user overestimate things and there would be just one segment within packet with
> PKT_TX_TCP_SEG - I don't think it is a big deal, things will keep working correctly and AFAIK
> there would be no slowdown.
>   
> That way it should probably simplify things for this feature and would help
> avoid setting extra ol_flags inside ipsec lib.

Yes, this sounds good, I will rework it like so.

> One side question - how PMD will report that this feature is supported?
> Would it be extra field in rte_security_ipsec_xform or something different?

The assumption is that if a PMD supports inline crypto and TSO it will 
support them together, as DEV_TX_OFFLOAD_SECURITY | DEV_TX_OFFLOAD_TCP_TSO