> 
> According to RFC791,the options may appear or not in datagrams.
> They must be implemented by all IP modules (host and gateways).
> What is optional is their transmission in any particular datagram,
> not their implementation.So we have to deal with it during the
> fragmenting process.Add some test data for the IPv4 header optional
> field fragmenting.
> 
> Signed-off-by: Huichao Cai <chcch...@163.com>
> ---

....

> diff --git a/lib/ip_frag/rte_ipv4_fragmentation.c 
> b/lib/ip_frag/rte_ipv4_fragmentation.c
> index 2e7739d..bcafa29 100644
> --- a/lib/ip_frag/rte_ipv4_fragmentation.c
> +++ b/lib/ip_frag/rte_ipv4_fragmentation.c
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: BSD-3-Clause
> +/* SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0)
>   * Copyright(c) 2010-2014 Intel Corporation
>   */
> 
> @@ -12,6 +12,13 @@
> 
>  #include "ip_frag_common.h"
> 
> +/* IP options */
> +#define RTE_IPOPT_COPY                               0x80
> +#define RTE_IPOPT_CONTROL                    0x00
> +#define RTE_IPOPT_END                                (0 | RTE_IPOPT_CONTROL)
> +#define RTE_IPOPT_NOOP                               (1 | RTE_IPOPT_CONTROL)
> +#define RTE_IPOPT_COPIED(o)                  ((o) & RTE_IPOPT_COPY)
> +
>  /* Fragment Offset */
>  #define      RTE_IPV4_HDR_DF_SHIFT                   14
>  #define      RTE_IPV4_HDR_MF_SHIFT                   13
> @@ -41,6 +48,38 @@ static inline void __free_fragments(struct rte_mbuf *mb[], 
> uint32_t num)
>               rte_pktmbuf_free(mb[i]);
>  }
> 
> +/*
> + *   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);

As a nit, why not 'uint8_t *', to keep style the same through all file? 

> +     int l = (iph->version_ihl & RTE_IPV4_HDR_IHL_MASK) *
> +         RTE_IPV4_IHL_MULTIPLIER - sizeof(struct rte_ipv4_hdr);

We already done such calculation in rte_ipv4_fragment_packet(),
so can re-use header_len value here.

> +     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)

Why optlen==1 is not considered as valid one?

> +                     return;
> +             if (!RTE_IPOPT_COPIED(*optptr))
> +                     memset(optptr, RTE_IPOPT_NOOP, optlen);
> +             l -= optlen;
> +             optptr += optlen;
> +     }
> +}
> +
>  /**
>   * IPv4 fragmentation.
>   *
> @@ -188,6 +227,17 @@ static inline void __free_fragments(struct rte_mbuf 
> *mb[], uint32_t num)
>                   (uint16_t)out_pkt->pkt_len,
>                   flag_offset, fragment_offset, more_in_segs);
> 
> +             /*
> +              * ANK:

What means 'ANK' here? 

> dirty, but effective trick. Upgrade options only if
> +              * the segment to be fragmented was THE FIRST (otherwise,
> +              * options are already fixed) and make it ONCE
> +              * on the initial mbuf, so that all the following fragments
> +              * will inherit fixed options.
> +              */
> +             if ((fragment_offset == 0) &&
> +                         ((flag_offset & RTE_IPV4_HDR_OFFSET_MASK) == 0))
> +                     ip_options_fragment(in_hdr);
> +

I see two problems with that approach:
- you modify incoming packet's header - which is the change in behaviour,
  and doesn't look right at all.
- you remove not-copied options from all fragments.
  As I can read RFC 791 - first fragment should have a copy of all options 
present
  in original packet, while other fragments need to have only those that have 
to be
  copied.  

>               fragment_offset = (uint16_t)(fragment_offset +
>                   out_pkt->pkt_len - header_len);
> 
> --
> 1.8.3.1

Reply via email to