[dpdk-dev] [PATCH 3/4] ether: add more protocol support in flow API

Pattan, Reshma reshma.pattan at intel.com
Thu Mar 29 16:32:22 CEST 2018


Hi,

> +
> +	/**
> +	 * Matches ARP IPv4 header.
> +	 *
> +	 * See struct rte_flow_item_arp.
> +	 */
> +	TE_FLOW_ITEM_TYPE_ARP,
> +

R is missing in name RTE_, similary in other places.

+	/**
+	 * Match ICMPv6 Source Link-Layer Address.
+	 *
+	 * See struct rte_flow_item_icmpv6_tll.
+	 */
+	RTE_FLOW_ITEM_TYPE_ICMPV6_TLL,

Description in comments not correct w.r.t enum name.
No need of ',' at the end.

> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_ARP. */ #ifndef __cplusplus
> +static const struct rte_flow_item_arp rte_flow_item_arp_mask = {
> +	.hdr = {
> +		.arp_data = {
> +			.arp_sha = {
> +			.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> +		},
> +		.arp_sip = RTE_BE32(0xffffffff),
> +		.arp_tha = {
> +			.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> +		},
> +			.arp_tip = RTE_BE32(0xffffffff),
> +		},
> +	},
> +};
> +#endif
> +

Indentation issue in above code.

> +/**
> + * RTE_FLOW_ITEM_TYPE_IPV6_EXT_HDR_ANY
> + *
> + * Matches any IPv6 extension header.
> + */
> +struct rte_flow_item_ipv6_ext_hdr_any {
> +	uint8_t next_hdr;
> +};
> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_IPV6_EXT_HDR_ANY. */
> #ifndef
> +__cplusplus static const struct rte_flow_item_ipv6_ext_hdr_any
> +rte_flow_item_ipv6_ext_any_mask = {
> +	.next_hdr = 0xff,
> +};
> +#endif

General comment, Is that ok to move all structure declarations together and 
all default mask settings inside one ifndef _cpluscplus?

> +
> +/** Default mask for RTE_FLOW_ITEM_TYPE_ICMPV6_TGT_ADDR */ #ifndef
> +__cplusplus static const struct rte_flow_item_icmpv6_tgt_addr
> +rte_flow_item_icmpv6_tgt_addr_mask = {
> +	.addr = "\xff\xff\xff\xff",

You should assign all 16bytes wilth xff? You are assigning only first 4.

Thanks,
Reshma


More information about the dev mailing list