Any review, please?
10/01/2019 16:06, Jiayu Hu:
> Currently, GRO library doesn't check if input packets have
> invalid headers. The packets with invalid headers will also
> be processed by GRO.
>
> However, GRO shouldn't process invalid packets. This patch adds
> missing invalid packet checks.
>
> Fixes: 0d2cbe59b719 ("lib/gro: support TCP/IPv4")
> Fixes: 9e0b9d2ec0f4 ("gro: support VxLAN GRO")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Jiayu Hu <jiayu...@intel.com>
> ---
> changes in v2:
> - fix VxLAN header length check bug for VxLAN GRO;
> - fix ethernet header length check bug;
> - use sizeof() and macro to present valid header length;
> - add VLAN related comments since GRO cannot process VLAN tagged packets.
>
> lib/librte_gro/gro_tcp4.c | 12 ++++++++++++
> lib/librte_gro/gro_tcp4.h | 10 ++++++++++
> lib/librte_gro/gro_vxlan_tcp4.c | 15 +++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/lib/librte_gro/gro_tcp4.c b/lib/librte_gro/gro_tcp4.c
> index 2fe9aab..48076e0 100644
> --- a/lib/librte_gro/gro_tcp4.c
> +++ b/lib/librte_gro/gro_tcp4.c
> @@ -208,6 +208,18 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,
> int cmp;
> uint8_t find;
>
> + /*
> + * Don't process the packet whose Ethernet, IPv4 and TCP header
> + * lengths are invalid.
> + *
> + * In addition, GRO doesn't process the packet that is VLAN
> + * tagged or whose the IPv4 header contains Options.
> + */
> + if (unlikely(ILLEGAL_ETHER_HDRLEN(pkt->l2_len) ||
> + ILLEGAL_IPV4_HDRLEN(pkt->l3_len) ||
> + ILLEGAL_TCP_HDRLEN(pkt->l4_len)))
> + return -1;
> +
> eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
> ipv4_hdr = (struct ipv4_hdr *)((char *)eth_hdr + pkt->l2_len);
> tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len);
> diff --git a/lib/librte_gro/gro_tcp4.h b/lib/librte_gro/gro_tcp4.h
> index 6bb30cd..65bcae8 100644
> --- a/lib/librte_gro/gro_tcp4.h
> +++ b/lib/librte_gro/gro_tcp4.h
> @@ -17,6 +17,16 @@
> */
> #define MAX_IPV4_PKT_LENGTH UINT16_MAX
>
> +/* The maximum TCP header length */
> +#define TCP_MAX_HLEN 60
> +
> +#define ILLEGAL_ETHER_HDRLEN(len) ((len) != ETHER_HDR_LEN)
> +#define ILLEGAL_ETHER_VXLAN_HDRLEN(len) \
> + ((len) != (ETHER_VXLAN_HLEN + ETHER_HDR_LEN))
> +#define ILLEGAL_IPV4_HDRLEN(len) ((len) != sizeof(struct ipv4_hdr))
> +#define ILLEGAL_TCP_HDRLEN(len) \
> + (((len) < sizeof(struct tcp_hdr)) || ((len) > TCP_MAX_HLEN))
> +
> /* Header fields representing a TCP/IPv4 flow */
> struct tcp4_flow_key {
> struct ether_addr eth_saddr;
> diff --git a/lib/librte_gro/gro_vxlan_tcp4.c b/lib/librte_gro/gro_vxlan_tcp4.c
> index 955ae4b..72d63bc 100644
> --- a/lib/librte_gro/gro_vxlan_tcp4.c
> +++ b/lib/librte_gro/gro_vxlan_tcp4.c
> @@ -306,6 +306,21 @@ gro_vxlan_tcp4_reassemble(struct rte_mbuf *pkt,
> uint16_t hdr_len;
> uint8_t find;
>
> + /*
> + * Don't process the packet whose outer Ethernet, outer IPv4,
> + * VxLAN header, inner Ethernet, inner IPv4 and inner TCP
> + * header lengths are invalid.
> + *
> + * In addition, GRO doesn't process the packet that is VLAN
> + * tagged or whose IPv4 header contains Options.
> + */
> + if (unlikely(ILLEGAL_ETHER_HDRLEN(pkt->outer_l2_len) ||
> + ILLEGAL_IPV4_HDRLEN(pkt->outer_l3_len) ||
> + ILLEGAL_ETHER_VXLAN_HDRLEN(pkt->l2_len) ||
> + ILLEGAL_IPV4_HDRLEN(pkt->l3_len) ||
> + ILLEGAL_TCP_HDRLEN(pkt->l4_len)))
> + return -1;
> +
> outer_eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
> outer_ipv4_hdr = (struct ipv4_hdr *)((char *)outer_eth_hdr +
> pkt->outer_l2_len);
>
