From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id DC5482BCE for ; Mon, 13 May 2019 13:54:47 +0200 (CEST) Received: from lfbn-lil-1-768-100.w81-254.abo.wanadoo.fr ([81.254.99.100] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hQ9aA-0001bJ-7Z; Mon, 13 May 2019 13:57:27 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 13 May 2019 13:54:39 +0200 Date: Mon, 13 May 2019 13:54:39 +0200 From: Olivier Matz To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: thomas@monjalon.net, ferruh.yigit@intel.com, arybchenko@solarflare.com, dev@dpdk.org Message-ID: <20190513115439.5fjarh6au7u64fmq@platinum> References: <1557509654-19325-1-git-send-email-mb@smartsharesystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1557509654-19325-1-git-send-email-mb@smartsharesystems.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] librte_net: define TCP flags in rte_tcp.h instead of in rte_eth_ctrl.h X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 May 2019 11:54:48 -0000 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 63B7AA00E6 for ; Mon, 13 May 2019 13:54:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 856884C9D; Mon, 13 May 2019 13:54:49 +0200 (CEST) Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id DC5482BCE for ; Mon, 13 May 2019 13:54:47 +0200 (CEST) Received: from lfbn-lil-1-768-100.w81-254.abo.wanadoo.fr ([81.254.99.100] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hQ9aA-0001bJ-7Z; Mon, 13 May 2019 13:57:27 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 13 May 2019 13:54:39 +0200 Date: Mon, 13 May 2019 13:54:39 +0200 From: Olivier Matz To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: thomas@monjalon.net, ferruh.yigit@intel.com, arybchenko@solarflare.com, dev@dpdk.org Message-ID: <20190513115439.5fjarh6au7u64fmq@platinum> References: <1557509654-19325-1-git-send-email-mb@smartsharesystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1557509654-19325-1-git-send-email-mb@smartsharesystems.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] librte_net: define TCP flags in rte_tcp.h instead of in rte_eth_ctrl.h X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190513115439.S-wexp1BQWhcpAD1JaruGqakycA3EwUSFI-8q49_4hA@z> 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