Hi Morten, On Fri, May 10, 2019 at 07:34:14PM +0200, Morten Brørup wrote: > TCP flags were moved to the TCP header file from the Ethernet control > header file, keeping their names intact. > > Missing TCP ECN flags were added. > > The ALL mask did not include TCP ECN flags, so it was renamed to reflect > that it applies to N-tuple filtering only. > A driver using the ALL mask was updated accordingly. > > Signed-off-by: Morten Brørup > --- > app/test-pmd/cmdline.c | 4 ++-- > drivers/net/e1000/igb_ethdev.c | 8 ++++---- > lib/librte_ethdev/rte_eth_ctrl.h | 8 +------- > lib/librte_net/rte_tcp.h | 13 +++++++++++++ > 4 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index ee50e45..59a45c1 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -9983,7 +9983,7 @@ cmd_2tuple_filter_parsed(void *parsed_result, > " when protocol is TCP.\n"); > return; > } > - if (res->tcp_flags_value > TCP_FLAG_ALL) { > + if (res->tcp_flags_value > RTE_NTUPLE_TCP_FLAGS_MASK) { > printf("invalid TCP flags.\n"); > return; > } > @@ -10141,7 +10141,7 @@ cmd_5tuple_filter_parsed(void *parsed_result, > " when protocol is TCP.\n"); > return; > } > - if (res->tcp_flags_value > TCP_FLAG_ALL) { > + if (res->tcp_flags_value > RTE_NTUPLE_TCP_FLAGS_MASK) { > printf("invalid TCP flags.\n"); > return; > } > diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c > index b897e8a..c2fe19a 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -3702,7 +3702,7 @@ ntuple_filter_to_2tuple(struct rte_eth_ntuple_filter *filter, > return -EINVAL; > if (filter->priority > E1000_2TUPLE_MAX_PRI) > return -EINVAL; /* filter index is out of range. */ > - if (filter->tcp_flags > TCP_FLAG_ALL) > + if (filter->tcp_flags > RTE_NTUPLE_TCP_FLAGS_MASK) > return -EINVAL; /* flags is invalid. */ > > switch (filter->dst_port_mask) { > @@ -3782,7 +3782,7 @@ igb_inject_2uple_filter(struct rte_eth_dev *dev, > ttqf &= ~E1000_TTQF_MASK_ENABLE; > > /* tcp flags bits setting. */ > - if (filter->filter_info.tcp_flags & TCP_FLAG_ALL) { > + if (filter->filter_info.tcp_flags & RTE_NTUPLE_TCP_FLAGS_MASK) { > if (filter->filter_info.tcp_flags & TCP_URG_FLAG) > imir_ext |= E1000_IMIREXT_CTRL_URG; > if (filter->filter_info.tcp_flags & TCP_ACK_FLAG) > @@ -4188,7 +4188,7 @@ ntuple_filter_to_5tuple_82576(struct rte_eth_ntuple_filter *filter, > return -EINVAL; > if (filter->priority > E1000_2TUPLE_MAX_PRI) > return -EINVAL; /* filter index is out of range. */ > - if (filter->tcp_flags > TCP_FLAG_ALL) > + if (filter->tcp_flags > RTE_NTUPLE_TCP_FLAGS_MASK) > return -EINVAL; /* flags is invalid. */ > > switch (filter->dst_ip_mask) { > @@ -4318,7 +4318,7 @@ igb_inject_5tuple_filter_82576(struct rte_eth_dev *dev, > imir |= filter->filter_info.priority << E1000_IMIR_PRIORITY_SHIFT; > > /* tcp flags bits setting. */ > - if (filter->filter_info.tcp_flags & TCP_FLAG_ALL) { > + if (filter->filter_info.tcp_flags & RTE_NTUPLE_TCP_FLAGS_MASK) { > if (filter->filter_info.tcp_flags & TCP_URG_FLAG) > imir_ext |= E1000_IMIREXT_CTRL_URG; > if (filter->filter_info.tcp_flags & TCP_ACK_FLAG) > diff --git a/lib/librte_ethdev/rte_eth_ctrl.h b/lib/librte_ethdev/rte_eth_ctrl.h > index 5ea8ae2..f21e84b 100644 > --- a/lib/librte_ethdev/rte_eth_ctrl.h > +++ b/lib/librte_ethdev/rte_eth_ctrl.h > @@ -184,13 +184,7 @@ struct rte_eth_syn_filter { > RTE_NTUPLE_FLAGS_DST_PORT | \ > RTE_NTUPLE_FLAGS_PROTO) > > -#define TCP_URG_FLAG 0x20 > -#define TCP_ACK_FLAG 0x10 > -#define TCP_PSH_FLAG 0x08 > -#define TCP_RST_FLAG 0x04 > -#define TCP_SYN_FLAG 0x02 > -#define TCP_FIN_FLAG 0x01 > -#define TCP_FLAG_ALL 0x3F > +#define RTE_NTUPLE_TCP_FLAGS_MASK 0x3F /**< TCP flags filter can match. */ > > /** > * A structure used to define the ntuple filter entry > diff --git a/lib/librte_net/rte_tcp.h b/lib/librte_net/rte_tcp.h > index 91f5898..2e89b5e 100644 > --- a/lib/librte_net/rte_tcp.h > +++ b/lib/librte_net/rte_tcp.h > @@ -2,6 +2,7 @@ > * Copyright(c) 1982, 1986, 1990, 1993 > * The Regents of the University of California. > * Copyright(c) 2010-2014 Intel Corporation. > + * Copyright(c) 2019 SmartShare Systems. > * All rights reserved. > */ > IMHO, adding the copyright for flags move is a bit overkill. > @@ -35,6 +36,18 @@ struct tcp_hdr { > uint16_t tcp_urp; /**< TCP urgent pointer, if any. */ > } __attribute__((__packed__)); > > +/** > + * TCP Flags > + */ > +#define TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */ > +#define TCP_ECE_FLAG 0x40 /**< ECN-Echo */ > +#define TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */ > +#define TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant */ > +#define TCP_PSH_FLAG 0x08 /**< Push Function */ > +#define TCP_RST_FLAG 0x04 /**< Reset the connection */ > +#define TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */ > +#define TCP_FIN_FLAG 0x01 /**< No more data from sender */ > + > #ifdef __cplusplus > } > #endif Moving these flags in rte_tcp.h makes sense. But I think they will need to be renamed with a RTE_ prefix inside this patchset: https://patches.dpdk.org/patch/52556/ So we need to take care to the dependency between these 2 patchsets. Acked-by: Olivier Matz