examples: remove Rx checksum offload
Checks
Commit Message
As of now, application does not check PKT_RX_*_CKSUM_* flags per
packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
cycles if possible.
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
examples/ip_fragmentation/main.c | 3 +--
examples/ip_reassembly/main.c | 3 +--
examples/ipsec-secgw/ipsec-secgw.c | 3 +--
examples/l3fwd-acl/main.c | 3 +--
examples/l3fwd-power/main.c | 4 ++--
examples/l3fwd-vf/main.c | 3 +--
examples/l3fwd/main.c | 3 +--
examples/load_balancer/init.c | 3 +--
examples/multi_process/symmetric_mp/main.c | 3 +--
examples/performance-thread/l3fwd-thread/main.c | 3 +--
examples/qos_meter/main.c | 3 +--
11 files changed, 12 insertions(+), 22 deletions(-)
Comments
Hi Jerin,
>
> As of now, application does not check PKT_RX_*_CKSUM_* flags per
> packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
>
> Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> cycles if possible.
Personally, I'd move in other direction: keep RX checksum offload and add
checks inside sample apps to handle (drop) packets with invalid checksum.
Konstantin
>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
> examples/ip_fragmentation/main.c | 3 +--
> examples/ip_reassembly/main.c | 3 +--
> examples/ipsec-secgw/ipsec-secgw.c | 3 +--
> examples/l3fwd-acl/main.c | 3 +--
> examples/l3fwd-power/main.c | 4 ++--
> examples/l3fwd-vf/main.c | 3 +--
> examples/l3fwd/main.c | 3 +--
> examples/load_balancer/init.c | 3 +--
> examples/multi_process/symmetric_mp/main.c | 3 +--
> examples/performance-thread/l3fwd-thread/main.c | 3 +--
> examples/qos_meter/main.c | 3 +--
> 11 files changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> index 5306d7672..7cbd5dc10 100644
> --- a/examples/ip_fragmentation/main.c
> +++ b/examples/ip_fragmentation/main.c
> @@ -140,8 +140,7 @@ static struct rte_eth_conf port_conf = {
> .rxmode = {
> .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_JUMBO_FRAME |
> + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> DEV_RX_OFFLOAD_CRC_STRIP),
> },
> .txmode = {
> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> index b830f67a5..7acc6b7b5 100644
> --- a/examples/ip_reassembly/main.c
> +++ b/examples/ip_reassembly/main.c
> @@ -164,8 +164,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_JUMBO_FRAME |
> + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> DEV_RX_OFFLOAD_CRC_STRIP),
> },
> .rx_adv_conf = {
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index b45b87bde..63eef1f26 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -197,8 +197,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_CRC_STRIP,
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
> index 7c063a8d0..c66b5b462 100644
> --- a/examples/l3fwd-acl/main.c
> +++ b/examples/l3fwd-acl/main.c
> @@ -127,8 +127,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> - DEV_RX_OFFLOAD_CHECKSUM),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index d15cd520e..b00a8ec45 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -180,8 +180,8 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> - DEV_RX_OFFLOAD_CHECKSUM),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> +
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/l3fwd-vf/main.c b/examples/l3fwd-vf/main.c
> index 5edd91a78..2a10e9d76 100644
> --- a/examples/l3fwd-vf/main.c
> +++ b/examples/l3fwd-vf/main.c
> @@ -161,8 +161,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> - DEV_RX_OFFLOAD_CHECKSUM),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index ab019b9e4..d4a79b23f 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -120,8 +120,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> - DEV_RX_OFFLOAD_CHECKSUM),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/load_balancer/init.c b/examples/load_balancer/init.c
> index f2045f235..14ea1e9d8 100644
> --- a/examples/load_balancer/init.c
> +++ b/examples/load_balancer/init.c
> @@ -45,8 +45,7 @@ static struct rte_eth_conf port_conf = {
> .rxmode = {
> .mq_mode = ETH_MQ_RX_RSS,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_CRC_STRIP),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
> index c6c6a537f..2735a68c5 100644
> --- a/examples/multi_process/symmetric_mp/main.c
> +++ b/examples/multi_process/symmetric_mp/main.c
> @@ -178,8 +178,7 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
> .rxmode = {
> .mq_mode = ETH_MQ_RX_RSS,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_CRC_STRIP),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/performance-thread/l3fwd-thread/main.c b/examples/performance-thread/l3fwd-thread/main.c
> index 5392fcea8..5f4a470b4 100644
> --- a/examples/performance-thread/l3fwd-thread/main.c
> +++ b/examples/performance-thread/l3fwd-thread/main.c
> @@ -306,8 +306,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_CRC_STRIP),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index 5cf4e9dfa..a29032b04 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -56,8 +56,7 @@ static struct rte_eth_conf port_conf = {
> .mq_mode = ETH_MQ_RX_RSS,
> .max_rx_pkt_len = ETHER_MAX_LEN,
> .split_hdr_size = 0,
> - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> - DEV_RX_OFFLOAD_CRC_STRIP),
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .rx_adv_conf = {
> .rss_conf = {
> --
> 2.18.0
-----Original Message-----
> Date: Mon, 30 Jul 2018 09:27:48 +0000
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "dev@dpdk.org"
> <dev@dpdk.org>
> CC: "thomas@monjalon.net" <thomas@monjalon.net>, "Yigit, Ferruh"
> <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>
>
> Hi Jerin,
Hi Konstantin,
>
> >
> > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> >
> > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > cycles if possible.
>
> Personally, I'd move in other direction: keep RX checksum offload and add
> checks inside sample apps to handle (drop) packets with invalid checksum.
OK. Till someones add the DROP logic in application, Can we take
this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
without DROP or any meaning full action in application.
> Konstantin
>
> >
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> > examples/ip_fragmentation/main.c | 3 +--
> > examples/ip_reassembly/main.c | 3 +--
> > examples/ipsec-secgw/ipsec-secgw.c | 3 +--
> > examples/l3fwd-acl/main.c | 3 +--
> > examples/l3fwd-power/main.c | 4 ++--
> > examples/l3fwd-vf/main.c | 3 +--
> > examples/l3fwd/main.c | 3 +--
> > examples/load_balancer/init.c | 3 +--
> > examples/multi_process/symmetric_mp/main.c | 3 +--
> > examples/performance-thread/l3fwd-thread/main.c | 3 +--
> > examples/qos_meter/main.c | 3 +--
> > 11 files changed, 12 insertions(+), 22 deletions(-)
> >
> > diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c
> > index 5306d7672..7cbd5dc10 100644
> > --- a/examples/ip_fragmentation/main.c
> > +++ b/examples/ip_fragmentation/main.c
> > @@ -140,8 +140,7 @@ static struct rte_eth_conf port_conf = {
> > .rxmode = {
> > .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_JUMBO_FRAME |
> > + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> > DEV_RX_OFFLOAD_CRC_STRIP),
> > },
> > .txmode = {
> > diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
> > index b830f67a5..7acc6b7b5 100644
> > --- a/examples/ip_reassembly/main.c
> > +++ b/examples/ip_reassembly/main.c
> > @@ -164,8 +164,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_JUMBO_FRAME |
> > + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> > DEV_RX_OFFLOAD_CRC_STRIP),
> > },
> > .rx_adv_conf = {
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> > index b45b87bde..63eef1f26 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -197,8 +197,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_CRC_STRIP,
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
> > index 7c063a8d0..c66b5b462 100644
> > --- a/examples/l3fwd-acl/main.c
> > +++ b/examples/l3fwd-acl/main.c
> > @@ -127,8 +127,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > - DEV_RX_OFFLOAD_CHECKSUM),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> > index d15cd520e..b00a8ec45 100644
> > --- a/examples/l3fwd-power/main.c
> > +++ b/examples/l3fwd-power/main.c
> > @@ -180,8 +180,8 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > - DEV_RX_OFFLOAD_CHECKSUM),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > +
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/l3fwd-vf/main.c b/examples/l3fwd-vf/main.c
> > index 5edd91a78..2a10e9d76 100644
> > --- a/examples/l3fwd-vf/main.c
> > +++ b/examples/l3fwd-vf/main.c
> > @@ -161,8 +161,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > - DEV_RX_OFFLOAD_CHECKSUM),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > index ab019b9e4..d4a79b23f 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -120,8 +120,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > - DEV_RX_OFFLOAD_CHECKSUM),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/load_balancer/init.c b/examples/load_balancer/init.c
> > index f2045f235..14ea1e9d8 100644
> > --- a/examples/load_balancer/init.c
> > +++ b/examples/load_balancer/init.c
> > @@ -45,8 +45,7 @@ static struct rte_eth_conf port_conf = {
> > .rxmode = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_CRC_STRIP),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
> > index c6c6a537f..2735a68c5 100644
> > --- a/examples/multi_process/symmetric_mp/main.c
> > +++ b/examples/multi_process/symmetric_mp/main.c
> > @@ -178,8 +178,7 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
> > .rxmode = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_CRC_STRIP),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/performance-thread/l3fwd-thread/main.c b/examples/performance-thread/l3fwd-thread/main.c
> > index 5392fcea8..5f4a470b4 100644
> > --- a/examples/performance-thread/l3fwd-thread/main.c
> > +++ b/examples/performance-thread/l3fwd-thread/main.c
> > @@ -306,8 +306,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_CRC_STRIP),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> > index 5cf4e9dfa..a29032b04 100644
> > --- a/examples/qos_meter/main.c
> > +++ b/examples/qos_meter/main.c
> > @@ -56,8 +56,7 @@ static struct rte_eth_conf port_conf = {
> > .mq_mode = ETH_MQ_RX_RSS,
> > .max_rx_pkt_len = ETHER_MAX_LEN,
> > .split_hdr_size = 0,
> > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > - DEV_RX_OFFLOAD_CRC_STRIP),
> > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > },
> > .rx_adv_conf = {
> > .rss_conf = {
> > --
> > 2.18.0
>
Monday, July 30, 2018 12:36 PM, Jerin Jacob:
> > Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> >
> >
> > Hi Jerin,
>
> Hi Konstantin,
>
> >
> > >
> > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or
> not.
> > >
> > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save
> a
> > > few cycles if possible.
> >
> > Personally, I'd move in other direction: keep RX checksum offload and
> > add checks inside sample apps to handle (drop) packets with invalid
> checksum.
>
> OK. Till someones add the DROP logic in application, Can we take this patch?
> Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM without
> DROP or any meaning full action in application.
I agree w/ Constantine.
If the l3fwd application targets to simulate vrouter which do IP forwarding validating the checksum is part of the required logic.
The patch should not remove it rather add check on the mbuf checksum fields and drop the mbuf accordingly.
>
>
> > Konstantin
> >
> > >
> > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > ---
> > > examples/ip_fragmentation/main.c | 3 +--
> > > examples/ip_reassembly/main.c | 3 +--
> > > examples/ipsec-secgw/ipsec-secgw.c | 3 +--
> > > examples/l3fwd-acl/main.c | 3 +--
> > > examples/l3fwd-power/main.c | 4 ++--
> > > examples/l3fwd-vf/main.c | 3 +--
> > > examples/l3fwd/main.c | 3 +--
> > > examples/load_balancer/init.c | 3 +--
> > > examples/multi_process/symmetric_mp/main.c | 3 +--
> > > examples/performance-thread/l3fwd-thread/main.c | 3 +--
> > > examples/qos_meter/main.c | 3 +--
> > > 11 files changed, 12 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/examples/ip_fragmentation/main.c
> > > b/examples/ip_fragmentation/main.c
> > > index 5306d7672..7cbd5dc10 100644
> > > --- a/examples/ip_fragmentation/main.c
> > > +++ b/examples/ip_fragmentation/main.c
> > > @@ -140,8 +140,7 @@ static struct rte_eth_conf port_conf = {
> > > .rxmode = {
> > > .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_JUMBO_FRAME |
> > > + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> > > DEV_RX_OFFLOAD_CRC_STRIP),
> > > },
> > > .txmode = {
> > > diff --git a/examples/ip_reassembly/main.c
> > > b/examples/ip_reassembly/main.c index b830f67a5..7acc6b7b5 100644
> > > --- a/examples/ip_reassembly/main.c
> > > +++ b/examples/ip_reassembly/main.c
> > > @@ -164,8 +164,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_JUMBO_FRAME |
> > > + .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
> > > DEV_RX_OFFLOAD_CRC_STRIP),
> > > },
> > > .rx_adv_conf = {
> > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > > b/examples/ipsec-secgw/ipsec-secgw.c
> > > index b45b87bde..63eef1f26 100644
> > > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > > @@ -197,8 +197,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_CRC_STRIP,
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
> > > index 7c063a8d0..c66b5b462 100644
> > > --- a/examples/l3fwd-acl/main.c
> > > +++ b/examples/l3fwd-acl/main.c
> > > @@ -127,8 +127,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > > - DEV_RX_OFFLOAD_CHECKSUM),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/l3fwd-power/main.c
> > > b/examples/l3fwd-power/main.c index d15cd520e..b00a8ec45 100644
> > > --- a/examples/l3fwd-power/main.c
> > > +++ b/examples/l3fwd-power/main.c
> > > @@ -180,8 +180,8 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > > - DEV_RX_OFFLOAD_CHECKSUM),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > +
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/l3fwd-vf/main.c b/examples/l3fwd-vf/main.c
> > > index 5edd91a78..2a10e9d76 100644
> > > --- a/examples/l3fwd-vf/main.c
> > > +++ b/examples/l3fwd-vf/main.c
> > > @@ -161,8 +161,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > > - DEV_RX_OFFLOAD_CHECKSUM),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c index
> > > ab019b9e4..d4a79b23f 100644
> > > --- a/examples/l3fwd/main.c
> > > +++ b/examples/l3fwd/main.c
> > > @@ -120,8 +120,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > > - DEV_RX_OFFLOAD_CHECKSUM),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/load_balancer/init.c
> > > b/examples/load_balancer/init.c index f2045f235..14ea1e9d8 100644
> > > --- a/examples/load_balancer/init.c
> > > +++ b/examples/load_balancer/init.c
> > > @@ -45,8 +45,7 @@ static struct rte_eth_conf port_conf = {
> > > .rxmode = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_CRC_STRIP),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/multi_process/symmetric_mp/main.c
> > > b/examples/multi_process/symmetric_mp/main.c
> > > index c6c6a537f..2735a68c5 100644
> > > --- a/examples/multi_process/symmetric_mp/main.c
> > > +++ b/examples/multi_process/symmetric_mp/main.c
> > > @@ -178,8 +178,7 @@ smp_port_init(uint16_t port, struct rte_mempool
> *mbuf_pool,
> > > .rxmode = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_CRC_STRIP),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = { diff --git
> > > a/examples/performance-thread/l3fwd-thread/main.c
> > > b/examples/performance-thread/l3fwd-thread/main.c
> > > index 5392fcea8..5f4a470b4 100644
> > > --- a/examples/performance-thread/l3fwd-thread/main.c
> > > +++ b/examples/performance-thread/l3fwd-thread/main.c
> > > @@ -306,8 +306,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_CRC_STRIP),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> > > index 5cf4e9dfa..a29032b04 100644
> > > --- a/examples/qos_meter/main.c
> > > +++ b/examples/qos_meter/main.c
> > > @@ -56,8 +56,7 @@ static struct rte_eth_conf port_conf = {
> > > .mq_mode = ETH_MQ_RX_RSS,
> > > .max_rx_pkt_len = ETHER_MAX_LEN,
> > > .split_hdr_size = 0,
> > > - .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
> > > - DEV_RX_OFFLOAD_CRC_STRIP),
> > > + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> > > },
> > > .rx_adv_conf = {
> > > .rss_conf = {
> > > --
> > > 2.18.0
> >
30/07/2018 11:35, Jerin Jacob:
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > >
> > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> > >
> > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > > cycles if possible.
> >
> > Personally, I'd move in other direction: keep RX checksum offload and add
> > checks inside sample apps to handle (drop) packets with invalid checksum.
>
> OK. Till someones add the DROP logic in application, Can we take
> this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
> without DROP or any meaning full action in application.
If there is no patch sent to use this offload on August 1st,
then I will apply this patch to remove the offload request.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, July 30, 2018 10:51 AM
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
> Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>
> 30/07/2018 11:35, Jerin Jacob:
> > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > > >
> > > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> > > >
> > > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > > > cycles if possible.
> > >
> > > Personally, I'd move in other direction: keep RX checksum offload and add
> > > checks inside sample apps to handle (drop) packets with invalid checksum.
> >
> > OK. Till someones add the DROP logic in application, Can we take
> > this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
> > without DROP or any meaning full action in application.
Probably, but at least it gives users a right estimation how long the proper
RX/TX routine would take.
From other side what the point to disable these flags now, if we know that
we are doing wrong thing and will have to re-enable them again in future?
>
> If there is no patch sent to use this offload on August 1st,
> then I will apply this patch to remove the offload request.
>
Isn't it too late to do such things right now?
We are in RC3 stage and doesn't look like a critical issue.
Konstantin
-----Original Message-----
> Date: Mon, 30 Jul 2018 11:00:02 +0000
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> To: Thomas Monjalon <thomas@monjalon.net>, Jerin Jacob
> <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh"
> <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>
> External Email
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Monday, July 30, 2018 10:51 AM
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
> > Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> >
> > 30/07/2018 11:35, Jerin Jacob:
> > > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > > > >
> > > > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> > > > >
> > > > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > > > > cycles if possible.
> > > >
> > > > Personally, I'd move in other direction: keep RX checksum offload and add
> > > > checks inside sample apps to handle (drop) packets with invalid checksum.
> > >
> > > OK. Till someones add the DROP logic in application, Can we take
> > > this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
> > > without DROP or any meaning full action in application.
>
> Probably, but at least it gives users a right estimation how long the proper
> RX/TX routine would take.
For estimation, application can add any flag they want in local setup.
It does not need to be upstream with out feature complete.
> From other side what the point to disable these flags now, if we know that
At least nicvf Rx routines are crafted based DEV_RX_OFFLOAD_CHECKSUM
flags. If driver Rx routine crafted such case it will be useful.
> we are doing wrong thing and will have to re-enable them again in future?
But it is not correct now either. Right?
>
> >
> > If there is no patch sent to use this offload on August 1st,
> > then I will apply this patch to remove the offload request.
> >
>
> Isn't it too late to do such things right now?
> We are in RC3 stage and doesn't look like a critical issue.
Yes. We can add it when have we proper fix. Currently, it signaling a wrong
interpretation to application.
> Konstantin
>
>
>
> -----Original Message-----
> > Date: Mon, 30 Jul 2018 11:00:02 +0000
> > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > To: Thomas Monjalon <thomas@monjalon.net>, Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh"
> > <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
> > Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> >
> > External Email
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Monday, July 30, 2018 10:51 AM
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
> > > Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> > >
> > > 30/07/2018 11:35, Jerin Jacob:
> > > > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > > > > >
> > > > > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > > > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> > > > > >
> > > > > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > > > > > cycles if possible.
> > > > >
> > > > > Personally, I'd move in other direction: keep RX checksum offload and add
> > > > > checks inside sample apps to handle (drop) packets with invalid checksum.
> > > >
> > > > OK. Till someones add the DROP logic in application, Can we take
> > > > this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
> > > > without DROP or any meaning full action in application.
> >
> > Probably, but at least it gives users a right estimation how long the proper
> > RX/TX routine would take.
>
> For estimation, application can add any flag they want in local setup.
> It does not need to be upstream with out feature complete.
>
> > From other side what the point to disable these flags now, if we know that
>
> At least nicvf Rx routines are crafted based DEV_RX_OFFLOAD_CHECKSUM
> flags. If driver Rx routine crafted such case it will be useful.
>
> > we are doing wrong thing and will have to re-enable them again in future?
>
> But it is not correct now either. Right?
Yes, right now invalid cksum information is simply ignored.
As you pointed - some PMD select RX routine based on checksum offload flags.
Yes, removing these flags might produce better performance numbers.
But from my perspective - it would be an artificial and temporary improvement,
as for l3fwd like apps we'll need to revert it back and add code to drop invalid packets.
Konstantin
>
> >
> > >
> > > If there is no patch sent to use this offload on August 1st,
> > > then I will apply this patch to remove the offload request.
> > >
> >
> > Isn't it too late to do such things right now?
> > We are in RC3 stage and doesn't look like a critical issue.
>
> Yes. We can add it when have we proper fix. Currently, it signaling a wrong
> interpretation to application.
>
>
> > Konstantin
> >
> >
-----Original Message-----
> Date: Mon, 30 Jul 2018 14:12:12 +0000
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
> "Yigit, Ferruh" <ferruh.yigit@intel.com>, "shahafs@mellanox.com"
> <shahafs@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>
>
> >
> > -----Original Message-----
> > > Date: Mon, 30 Jul 2018 11:00:02 +0000
> > > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > > To: Thomas Monjalon <thomas@monjalon.net>, Jerin Jacob
> > > <jerin.jacob@caviumnetworks.com>
> > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh"
> > > <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
> > > Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> > >
> > > External Email
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > Sent: Monday, July 30, 2018 10:51 AM
> > > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
> > > > Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
> > > >
> > > > 30/07/2018 11:35, Jerin Jacob:
> > > > > From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> > > > > > >
> > > > > > > As of now, application does not check PKT_RX_*_CKSUM_* flags per
> > > > > > > packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
> > > > > > >
> > > > > > > Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
> > > > > > > cycles if possible.
> > > > > >
> > > > > > Personally, I'd move in other direction: keep RX checksum offload and add
> > > > > > checks inside sample apps to handle (drop) packets with invalid checksum.
> > > > >
> > > > > OK. Till someones add the DROP logic in application, Can we take
> > > > > this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
> > > > > without DROP or any meaning full action in application.
> > >
> > > Probably, but at least it gives users a right estimation how long the proper
> > > RX/TX routine would take.
> >
> > For estimation, application can add any flag they want in local setup.
> > It does not need to be upstream with out feature complete.
> >
> > > From other side what the point to disable these flags now, if we know that
> >
> > At least nicvf Rx routines are crafted based DEV_RX_OFFLOAD_CHECKSUM
> > flags. If driver Rx routine crafted such case it will be useful.
> >
> > > we are doing wrong thing and will have to re-enable them again in future?
> >
> > But it is not correct now either. Right?
>
> Yes, right now invalid cksum information is simply ignored.
> As you pointed - some PMD select RX routine based on checksum offload flags.
> Yes, removing these flags might produce better performance numbers.
> But from my perspective - it would be an artificial and temporary improvement,
> as for l3fwd like apps we'll need to revert it back and add code to drop invalid packets.
IMO, It is OK get a performance hit when do that support in l3fwd. There
is no harm in removing the DEV_RX_OFFLOAD_CHECKSUM flag for now and it
is correct from application perspective.(you are enabling an offload when
you are using it, else don't enable it. I believe, this was philosophy for
enabling Rx/Tx offloads)
Since it is going in circles, I leave decision to ethdev maintainers.
> Konstantin
>
> >
> > >
> > > >
> > > > If there is no patch sent to use this offload on August 1st,
> > > > then I will apply this patch to remove the offload request.
> > > >
> > >
> > > Isn't it too late to do such things right now?
> > > We are in RC3 stage and doesn't look like a critical issue.
> >
> > Yes. We can add it when have we proper fix. Currently, it signaling a wrong
> > interpretation to application.
> >
> >
> > > Konstantin
> > >
> > >
On 30.07.2018 17:40, Jerin Jacob wrote:
> -----Original Message-----
>> Date: Mon, 30 Jul 2018 14:12:12 +0000
>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> CC: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
>> "Yigit, Ferruh" <ferruh.yigit@intel.com>, "shahafs@mellanox.com"
>> <shahafs@mellanox.com>
>> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>
>>
>>> -----Original Message-----
>>>> Date: Mon, 30 Jul 2018 11:00:02 +0000
>>>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>>>> To: Thomas Monjalon <thomas@monjalon.net>, Jerin Jacob
>>>> <jerin.jacob@caviumnetworks.com>
>>>> CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh"
>>>> <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
>>>> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>>>
>>>> External Email
>>>>
>>>>> -----Original Message-----
>>>>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>>>>> Sent: Monday, July 30, 2018 10:51 AM
>>>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
>>>>> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
>>>>> Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>>>>
>>>>> 30/07/2018 11:35, Jerin Jacob:
>>>>>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>>>>>>>> As of now, application does not check PKT_RX_*_CKSUM_* flags per
>>>>>>>> packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
>>>>>>>>
>>>>>>>> Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
>>>>>>>> cycles if possible.
>>>>>>> Personally, I'd move in other direction: keep RX checksum offload and add
>>>>>>> checks inside sample apps to handle (drop) packets with invalid checksum.
>>>>>> OK. Till someones add the DROP logic in application, Can we take
>>>>>> this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
>>>>>> without DROP or any meaning full action in application.
>>>> Probably, but at least it gives users a right estimation how long the proper
>>>> RX/TX routine would take.
>>> For estimation, application can add any flag they want in local setup.
>>> It does not need to be upstream with out feature complete.
>>>
>>>> From other side what the point to disable these flags now, if we know that
>>> At least nicvf Rx routines are crafted based DEV_RX_OFFLOAD_CHECKSUM
>>> flags. If driver Rx routine crafted such case it will be useful.
>>>
>>>> we are doing wrong thing and will have to re-enable them again in future?
>>> But it is not correct now either. Right?
>> Yes, right now invalid cksum information is simply ignored.
>> As you pointed - some PMD select RX routine based on checksum offload flags.
>> Yes, removing these flags might produce better performance numbers.
>> But from my perspective - it would be an artificial and temporary improvement,
>> as for l3fwd like apps we'll need to revert it back and add code to drop invalid packets.
> IMO, It is OK get a performance hit when do that support in l3fwd. There
> is no harm in removing the DEV_RX_OFFLOAD_CHECKSUM flag for now and it
> is correct from application perspective.(you are enabling an offload when
> you are using it, else don't enable it. I believe, this was philosophy for
> enabling Rx/Tx offloads)
>
> Since it is going in circles, I leave decision to ethdev maintainers.
I think that IPv4 checksum offload is essential for l3fwd. So, it should be
enabled and taken into account. I'm not sure about TCP and UDP checksum
offloads. It is not l3fwd business to take a look at upper layers.
In any case, there is no agreement on the patch and it is already RC3 stage
of the release. There is no rush to apply it since it is not a critical
bug fix.
I agree with Konstantin here.
Andrew
>> Konstantin
>>
>>>>> If there is no patch sent to use this offload on August 1st,
>>>>> then I will apply this patch to remove the offload request.
>>>>>
>>>> Isn't it too late to do such things right now?
>>>> We are in RC3 stage and doesn't look like a critical issue.
>>> Yes. We can add it when have we proper fix. Currently, it signaling a wrong
>>> interpretation to application.
>>>
>>>
>>>> Konstantin
>>>>
>>>>
On 7/30/2018 4:30 PM, Andrew Rybchenko wrote:
> On 30.07.2018 17:40, Jerin Jacob wrote:
>> -----Original Message-----
>>> Date: Mon, 30 Jul 2018 14:12:12 +0000
>>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>> CC: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
>>> "Yigit, Ferruh" <ferruh.yigit@intel.com>, "shahafs@mellanox.com"
>>> <shahafs@mellanox.com>
>>> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>>
>>>
>>>> -----Original Message-----
>>>>> Date: Mon, 30 Jul 2018 11:00:02 +0000
>>>>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>>>>> To: Thomas Monjalon <thomas@monjalon.net>, Jerin Jacob
>>>>> <jerin.jacob@caviumnetworks.com>
>>>>> CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh"
>>>>> <ferruh.yigit@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>
>>>>> Subject: RE: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>>>>
>>>>> External Email
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>>>>>> Sent: Monday, July 30, 2018 10:51 AM
>>>>>> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
>>>>>> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; shahafs@mellanox.com
>>>>>> Subject: Re: [dpdk-dev] [PATCH] examples: remove Rx checksum offload
>>>>>>
>>>>>> 30/07/2018 11:35, Jerin Jacob:
>>>>>>> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
>>>>>>>>> As of now, application does not check PKT_RX_*_CKSUM_* flags per
>>>>>>>>> packet, so it does not matter DEV_RX_OFFLOAD_CHECKSUM enabled or not.
>>>>>>>>>
>>>>>>>>> Removing DEV_RX_OFFLOAD_CHECKSUM offload so that driver can save a few
>>>>>>>>> cycles if possible.
>>>>>>>> Personally, I'd move in other direction: keep RX checksum offload and add
>>>>>>>> checks inside sample apps to handle (drop) packets with invalid checksum.
>>>>>>> OK. Till someones add the DROP logic in application, Can we take
>>>>>>> this patch? Because there is no point in enabling DEV_RX_OFFLOAD_CHECKSUM
>>>>>>> without DROP or any meaning full action in application.
>>>>> Probably, but at least it gives users a right estimation how long the proper
>>>>> RX/TX routine would take.
>>>> For estimation, application can add any flag they want in local setup.
>>>> It does not need to be upstream with out feature complete.
>>>>
>>>>> From other side what the point to disable these flags now, if we know that
>>>> At least nicvf Rx routines are crafted based DEV_RX_OFFLOAD_CHECKSUM
>>>> flags. If driver Rx routine crafted such case it will be useful.
>>>>
>>>>> we are doing wrong thing and will have to re-enable them again in future?
>>>> But it is not correct now either. Right?
>>> Yes, right now invalid cksum information is simply ignored.
>>> As you pointed - some PMD select RX routine based on checksum offload flags.
>>> Yes, removing these flags might produce better performance numbers.
>>> But from my perspective - it would be an artificial and temporary improvement,
>>> as for l3fwd like apps we'll need to revert it back and add code to drop invalid packets.
>> IMO, It is OK get a performance hit when do that support in l3fwd. There
>> is no harm in removing the DEV_RX_OFFLOAD_CHECKSUM flag for now and it
>> is correct from application perspective.(you are enabling an offload when
>> you are using it, else don't enable it. I believe, this was philosophy for
>> enabling Rx/Tx offloads)
>>
>> Since it is going in circles, I leave decision to ethdev maintainers.
>
> I think that IPv4 checksum offload is essential for l3fwd. So, it should be
> enabled and taken into account. I'm not sure about TCP and UDP checksum
> offloads. It is not l3fwd business to take a look at upper layers.
>
> In any case, there is no agreement on the patch and it is already RC3 stage
> of the release. There is no rush to apply it since it is not a critical
> bug fix.
> I agree with Konstantin here.
Now it is in the scope of v18.11,
+1 to implement PKT_RX_*_CKSUM checks in applications instead of removing
DEV_RX_OFFLOAD_CHECKSUM
>
> Andrew
>
>>> Konstantin
>>>
>>>>>> If there is no patch sent to use this offload on August 1st,
>>>>>> then I will apply this patch to remove the offload request.
>>>>>>
>>>>> Isn't it too late to do such things right now?
>>>>> We are in RC3 stage and doesn't look like a critical issue.
>>>> Yes. We can add it when have we proper fix. Currently, it signaling a wrong
>>>> interpretation to application.
>>>>
>>>>
>>>>> Konstantin
>>>>>
>>>>>
>
@@ -140,8 +140,7 @@ static struct rte_eth_conf port_conf = {
.rxmode = {
.max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_JUMBO_FRAME |
+ .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
DEV_RX_OFFLOAD_CRC_STRIP),
},
.txmode = {
@@ -164,8 +164,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_JUMBO_FRAME |
+ .offloads = (DEV_RX_OFFLOAD_JUMBO_FRAME |
DEV_RX_OFFLOAD_CRC_STRIP),
},
.rx_adv_conf = {
@@ -197,8 +197,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_CRC_STRIP,
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -127,8 +127,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
- DEV_RX_OFFLOAD_CHECKSUM),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -180,8 +180,8 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
- DEV_RX_OFFLOAD_CHECKSUM),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
+
},
.rx_adv_conf = {
.rss_conf = {
@@ -161,8 +161,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
- DEV_RX_OFFLOAD_CHECKSUM),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -120,8 +120,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
- DEV_RX_OFFLOAD_CHECKSUM),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -45,8 +45,7 @@ static struct rte_eth_conf port_conf = {
.rxmode = {
.mq_mode = ETH_MQ_RX_RSS,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_CRC_STRIP),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -178,8 +178,7 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
.rxmode = {
.mq_mode = ETH_MQ_RX_RSS,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_CRC_STRIP),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -306,8 +306,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_CRC_STRIP),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {
@@ -56,8 +56,7 @@ static struct rte_eth_conf port_conf = {
.mq_mode = ETH_MQ_RX_RSS,
.max_rx_pkt_len = ETHER_MAX_LEN,
.split_hdr_size = 0,
- .offloads = (DEV_RX_OFFLOAD_CHECKSUM |
- DEV_RX_OFFLOAD_CRC_STRIP),
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.rx_adv_conf = {
.rss_conf = {