Hi Olivier, Thank you for responding to this, and sorry about the noise with my initial failed commit attempts.
> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Olivier Matz > Sent: Monday, May 13, 2019 1:55 PM > To: Morten Brørup > Cc: tho...@monjalon.net; ferruh.yi...@intel.com; > arybche...@solarflare.com; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] librte_net: define TCP flags in > rte_tcp.h instead of in rte_eth_ctrl.h > > 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 <m...@smartsharesystems.com> > > --- > > 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(-) > > > 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. I also added the two ECN flags and descriptions of all the flags. And removed the misleading ALL flag. I don't know the formal or customary threshold criteria for adding a copyright notice, so I just added it. It's our first direct contribution where we didn't just send the corrections to another contributor, so I thought it would be nice to have our name in here. You're the maintainer, so I'll leave the final decision up to you. > > > @@ -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. Agreed. I assume someone more git-experienced than myself can take care of this. > > Acked-by: Olivier Matz <olivier.m...@6wind.com> Med venlig hilsen / kind regards - Morten Brørup
