Re: [PATCH] ip_frag: add IPv4 options fragment and unit test data

[Dariusz Sosnowski <dsosnowski@nvidia.com>]

On Wed, 24 Nov 2021 16:47:06 +0800, Huichao Cai wrote:
> +/*
> + *     Options "fragmenting", just fill options not
> + *     allowed in fragments with NOOPs.
> + *     Simple and stupid 8), but the most efficient way.
> + */
> +static inline void ip_options_fragment(struct rte_ipv4_hdr *iph)
> +{
> +       unsigned char *optptr = (unsigned char *)iph +
> +           sizeof(struct rte_ipv4_hdr);
> +       int l = (iph->version_ihl & RTE_IPV4_HDR_IHL_MASK) *
> +           RTE_IPV4_IHL_MULTIPLIER - sizeof(struct rte_ipv4_hdr);
> +       int optlen;
> +
> +       while (l > 0) {
> +               switch (*optptr) {
> +               case RTE_IPOPT_END:
> +                       return;
> +               case RTE_IPOPT_NOOP:
> +                       l--;
> +                       optptr++;
> +                       continue;
> +               }
> +               optlen = optptr[1];
> +               if (optlen < 2 || optlen > l)
> +                       return;
> +               if (!RTE_IPOPT_COPIED(*optptr))
> +                       memset(optptr, RTE_IPOPT_NOOP, optlen);
> +               l -= optlen;
> +               optptr += optlen;
> +       }
> +}
> +
I have a few concerns regarding this implementation:
- Any IPv4 option longer than 2 bytes with copied flag unset, will not be substituted by NOOP option. In effect it will be copied to all fragments.
- Substituting options with NOOP might cause rte_ipv4_fragment_packet to produce more fragments than necessary, since options with copied flag unset will still occupy space in IPv4 header.
It would require some benchmarking, but maybe a better solution would be to prepare a separate IPv4 header for fragments without unnecessary options.