[dpdk-dev,v2,15/39] examples/ipsec-secgw: convert to new ethdev offloads API

Message ID 2cde002e4430cdc6ad44d6d1801994171b7a8340.1513081087.git.shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Shahaf Shuler Dec. 12, 2017, 12:26 p.m. UTC
  Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
  

Comments

De Lara Guarch, Pablo Dec. 19, 2017, 12:38 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Tuesday, December 12, 2017 12:27 PM
> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> Subject: [dpdk-dev] [PATCH v2 15/39] examples/ipsec-secgw: convert to
> new ethdev offloads API
> 
> Ethdev offloads API has changed since:
> 
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> 
> This commit support the new API.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> secgw/ipsec-secgw.c
> index c98454a..1e8af8d 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -217,6 +217,9 @@ struct lcore_conf {
>  	},
>  	.txmode = {
>  		.mq_mode = ETH_MQ_TX_NONE,
> +		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
> +			     DEV_TX_OFFLOAD_MULTI_SEGS |
> +			     DEV_TX_OFFLOAD_MBUF_FAST_FREE),

Hi Shahaf,

Isn't this removing some checksums that were previously done?
Txq_flags was set to 0, which means that SCTP, UDP... checksums
are disabled now?

Regards,
Pablo
  
Shahaf Shuler Dec. 21, 2017, 1:45 p.m. UTC | #2
Hi Pablo and maintainers of ipsec-secgw,

Tuesday, December 19, 2017 2:39 PM, De Lara Guarch, Pablo
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> > secgw/ipsec-secgw.c index c98454a..1e8af8d 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -217,6 +217,9 @@ struct lcore_conf {
> >  	},
> >  	.txmode = {
> >  		.mq_mode = ETH_MQ_TX_NONE,
> > +		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
> > +			     DEV_TX_OFFLOAD_MULTI_SEGS |
> > +			     DEV_TX_OFFLOAD_MBUF_FAST_FREE),
> 
> Hi Shahaf,
> 
> Isn't this removing some checksums that were previously done?
> Txq_flags was set to 0, which means that SCTP, UDP... checksums are
> disabled now?

You are right that before txqflags were 0, but it doesn't seem the application uses any Tx checksum offload beside IPv4, as seen on snipped code[1].
If I was mistaken and it does uses L4 checksums then I will need to update this commit. 

Maintainers of this examples - can you confirm?

[1]
static inline void                                                           
prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port)                          
{                                                                            
        struct ip *ip;                                                       
        struct ether_hdr *ethhdr;                                            
                                                                             
        ip = rte_pktmbuf_mtod(pkt, struct ip *);                             
                                                                             
        ethhdr = (struct ether_hdr *)rte_pktmbuf_prepend(pkt, ETHER_HDR_LEN);
                                                                             
        if (ip->ip_v == IPVERSION) {                                         
                pkt->ol_flags |= PKT_TX_IP_CKSUM | PKT_TX_IPV4;              
                pkt->l3_len = sizeof(struct ip);                             
                pkt->l2_len = ETHER_HDR_LEN;                                 
                                                                             
                ethhdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);      
        } else {                                                             
                pkt->ol_flags |= PKT_TX_IPV6;                                
                pkt->l3_len = sizeof(struct ip6_hdr);                        
                pkt->l2_len = ETHER_HDR_LEN;                                 
                                                                             
                ethhdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv6);      
        }                                                                    
                                                                             
        memcpy(&ethhdr->s_addr, &ethaddr_tbl[port].src,                      
                        sizeof(struct ether_addr));                          
        memcpy(&ethhdr->d_addr, &ethaddr_tbl[port].dst,                      
                        sizeof(struct ether_addr));                          
}                                                                            

> 
> Regards,
> Pablo
>
  
De Lara Guarch, Pablo Jan. 8, 2018, 4:27 p.m. UTC | #3
> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Thursday, December 21, 2017 1:45 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> Subject: RE: [dpdk-dev] [PATCH v2 15/39] examples/ipsec-secgw: convert to
> new ethdev offloads API
> 
> Hi Pablo and maintainers of ipsec-secgw,
> 
> Tuesday, December 19, 2017 2:39 PM, De Lara Guarch, Pablo
> > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> > > secgw/ipsec-secgw.c index c98454a..1e8af8d 100644
> > > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > > @@ -217,6 +217,9 @@ struct lcore_conf {
> > >  	},
> > >  	.txmode = {
> > >  		.mq_mode = ETH_MQ_TX_NONE,
> > > +		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
> > > +			     DEV_TX_OFFLOAD_MULTI_SEGS |
> > > +			     DEV_TX_OFFLOAD_MBUF_FAST_FREE),
> >
> > Hi Shahaf,
> >
> > Isn't this removing some checksums that were previously done?
> > Txq_flags was set to 0, which means that SCTP, UDP... checksums are
> > disabled now?
> 
> You are right that before txqflags were 0, but it doesn't seem the
> application uses any Tx checksum offload beside IPv4, as seen on snipped
> code[1].
> If I was mistaken and it does uses L4 checksums then I will need to update
> this commit.
> 
> Maintainers of this examples - can you confirm?
> 
Akhil, Radu, could you confirm that this change is OK for the IPSec app?
There is a v3 already of this patch: http://dpdk.org/dev/patchwork/patch/32711/

Thanks,
Pablo
  
Akhil Goyal Jan. 9, 2018, 7:07 a.m. UTC | #4
Hi Pablo,

On 1/8/2018 9:57 PM, De Lara Guarch, Pablo wrote:
> 
> 
>> -----Original Message-----
>> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
>> Sent: Thursday, December 21, 2017 1:45 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
>> Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
>> Subject: RE: [dpdk-dev] [PATCH v2 15/39] examples/ipsec-secgw: convert to
>> new ethdev offloads API
>>
>> Hi Pablo and maintainers of ipsec-secgw,
>>
>> Tuesday, December 19, 2017 2:39 PM, De Lara Guarch, Pablo
>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
>>>> secgw/ipsec-secgw.c index c98454a..1e8af8d 100644
>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>> @@ -217,6 +217,9 @@ struct lcore_conf {
>>>>   	},
>>>>   	.txmode = {
>>>>   		.mq_mode = ETH_MQ_TX_NONE,
>>>> +		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
>>>> +			     DEV_TX_OFFLOAD_MULTI_SEGS |
>>>> +			     DEV_TX_OFFLOAD_MBUF_FAST_FREE),
>>>
>>> Hi Shahaf,
>>>
>>> Isn't this removing some checksums that were previously done?
>>> Txq_flags was set to 0, which means that SCTP, UDP... checksums are
>>> disabled now?
>>
>> You are right that before txqflags were 0, but it doesn't seem the
>> application uses any Tx checksum offload beside IPv4, as seen on snipped
>> code[1].
>> If I was mistaken and it does uses L4 checksums then I will need to update
>> this commit.
>>
>> Maintainers of this examples - can you confirm?
>>
> Akhil, Radu, could you confirm that this change is OK for the IPSec app?
> There is a v3 already of this patch: http://dpdk.org/dev/patchwork/patch/32711/
> 
> Thanks,
> Pablo
> 
I believe Radu is a better person to review this one. It is related to 
ethernet offloads.

-Akhil
  

Patch

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index c98454a..1e8af8d 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -217,6 +217,9 @@  struct lcore_conf {
 	},
 	.txmode = {
 		.mq_mode = ETH_MQ_TX_NONE,
+		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
+			     DEV_TX_OFFLOAD_MULTI_SEGS |
+			     DEV_TX_OFFLOAD_MBUF_FAST_FREE),
 	},
 };
 
@@ -1394,6 +1397,20 @@  struct ipsec_traffic {
 	if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
 		port_conf.txmode.offloads |= DEV_TX_OFFLOAD_SECURITY;
 
+	if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
+	    port_conf.rxmode.offloads) {
+		printf("Some Rx offloads are not supported "
+		       "by port %d: requested 0x%lx supported 0x%lx\n",
+		       portid, port_conf.rxmode.offloads,
+		       dev_info.rx_offload_capa);
+	}
+	if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
+	     port_conf.txmode.offloads) {
+		printf("Some Tx offloads are not supported "
+		       "by port %d: requested 0x%lx supported 0x%lx\n",
+		       portid, port_conf.txmode.offloads,
+		       dev_info.tx_offload_capa);
+	}
 	ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue,
 			&port_conf);
 	if (ret < 0)
@@ -1420,7 +1437,8 @@  struct ipsec_traffic {
 		printf("Setup txq=%u,%d,%d\n", lcore_id, tx_queueid, socket_id);
 
 		txconf = &dev_info.default_txconf;
-		txconf->txq_flags = 0;
+		txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;
+		txconf->offloads = port_conf.txmode.offloads;
 
 		ret = rte_eth_tx_queue_setup(portid, tx_queueid, nb_txd,
 				socket_id, txconf);
@@ -1434,6 +1452,8 @@  struct ipsec_traffic {
 
 		/* init RX queues */
 		for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
+			struct rte_eth_rxconf rxq_conf;
+
 			if (portid != qconf->rx_queue_list[queue].port_id)
 				continue;
 
@@ -1442,8 +1462,10 @@  struct ipsec_traffic {
 			printf("Setup rxq=%d,%d,%d\n", portid, rx_queueid,
 					socket_id);
 
+			rxq_conf = dev_info.default_rxconf;
+			rxq_conf.offloads = port_conf.rxmode.offloads;
 			ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-					nb_rxd,	socket_id, NULL,
+					nb_rxd,	socket_id, &rxq_conf,
 					socket_ctx[socket_id].mbuf_pool);
 			if (ret < 0)
 				rte_exit(EXIT_FAILURE,