From: Radu Nicolau <radu.nicolau@intel.com>
To: Shahaf Shuler <shahafs@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 15/39] examples/ipsec-secgw: convert to new ethdev offloads API
Date: Mon, 11 Dec 2017 12:51:14 +0000 [thread overview]
Message-ID: <cc4858f5-d1e5-0326-df11-68cfa81ea64d@intel.com> (raw)
In-Reply-To: <VI1PR05MB3149DF8987224ADACC07A7F8C3370@VI1PR05MB3149.eurprd05.prod.outlook.com>
On 12/11/2017 12:33 PM, Shahaf Shuler wrote:
> Hi Radu,
>
> Monday, December 11, 2017 1:48 PM, Radu Nicolau :
>> Hi,
>>
>> Comment inline
>>
>>
>> On 11/23/2017 12:19 PM, Shahaf Shuler wrote:
>>> 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 | 27
>> +++++++++++++++++++++++++--
>>> 1 file changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c
>>> b/examples/ipsec-secgw/ipsec-secgw.c
>>> index c98454a90..6e538a1ab 100644
>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>> @@ -217,6 +217,8 @@ static struct rte_eth_conf port_conf = {
>>> },
>>> .txmode = {
>>> .mq_mode = ETH_MQ_TX_NONE,
>>> + .offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
>>> + DEV_TX_OFFLOAD_MULTI_SEGS),
>>> },
>>> };
>>>
>>> @@ -1394,6 +1396,22 @@ port_init(uint16_t portid)
>>> 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);
>>> + 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);
>>> + port_conf.txmode.offloads &= dev_info.tx_offload_capa;
>>> + }
>> I don't think that clearing the offload flags that are not advertised in the
>> capabilities is a good approach, although it may be the right one.
>> From what I can see there are more PMDs that don't fully populate the
>> offload capabilities, but actually check for them in the configure/start
>> function. One of them is ixgbe, which needs CRC strip enabled when IPSec is
>> enabled, and will fail to start otherwise. So although it supports CRC strip it
>> does not set the flag in the capabilities, but checks it in the start function.
> Why ixgbe don't expose the CRC cap then? It seems wrong behavior to expect the application to set it without any cap reported.
It is bad behavior but from what I can see most, if not all, PMDs don't
expose CRC strip (or jumbo frames) while still supporting it.
>
>> I would propose to just print a warning if a requested offload is not set in the
>> capabilities, but let the pmd start fail if it is not really supported.
>
> I think I agree, however not from the reason you mentioned.
> It is bad to mask the un-supported offloads because the application relies on them to be set successfully. The application will not run successfully if the IPV4 checksum is not actually set (for example).
>
> On v2 I will print just the warn.
>
>>> ret = rte_eth_dev_configure(portid, nb_rx_queue, nb_tx_queue,
>>> &port_conf);
>>> if (ret < 0)
>>> @@ -1420,7 +1438,8 @@ port_init(uint16_t portid)
>>> 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 +1453,8 @@ port_init(uint16_t portid)
>>>
>>> /* 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 +1463,10 @@ port_init(uint16_t portid)
>>> 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,
next prev parent reply other threads:[~2017-12-11 12:51 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-23 12:19 [dpdk-dev] [PATCH 10/39] examples/exception_path: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 11/39] examples/kni: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 12/39] examples/ip_fragmentation: convert to new " Shahaf Shuler
2017-12-11 14:56 ` Ananyev, Konstantin
2017-11-23 12:19 ` [dpdk-dev] [PATCH 13/39] examples/ip_pipeline: convert to new ethdev " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 14/39] examples/ip_reassembly: " Shahaf Shuler
2017-12-11 15:03 ` Ananyev, Konstantin
2017-12-12 6:30 ` Shahaf Shuler
2017-12-12 8:49 ` Ananyev, Konstantin
2017-11-23 12:19 ` [dpdk-dev] [PATCH 15/39] examples/ipsec-secgw: " Shahaf Shuler
2017-12-11 11:47 ` Radu Nicolau
2017-12-11 12:33 ` Shahaf Shuler
2017-12-11 12:51 ` Radu Nicolau [this message]
2017-11-23 12:19 ` [dpdk-dev] [PATCH 16/39] examples/ipv4_multicast: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 17/39] examples/link_status_interrupt: convert to new " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 18/39] examples/load_balancer: convert to new ethdev " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 19/39] examples/multi_process: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 20/39] examples/netmap_compat: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 21/39] examples/performance-thread: convert to new " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 22/39] examples/qos_meter: convert to new ethdev " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 23/39] examples/qos_sched: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 24/39] examples/quota_watermark: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 25/39] examples/tep_termination: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 26/39] examples/vhost: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 27/39] examples/vmdq: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 28/39] examples/vmdq_dcb: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 29/39] examples/vm_power_manager: convert to new " Shahaf Shuler
2017-12-11 12:06 ` Hunt, David
2017-11-23 12:19 ` [dpdk-dev] [PATCH 30/39] examples/distributor: convert to new ethdev " Shahaf Shuler
2017-12-11 16:20 ` Bruce Richardson
2017-11-23 12:19 ` [dpdk-dev] [PATCH 31/39] examples/ethtool: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 32/39] examples/eventdev_pipeline: convert to new " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 33/39] examples/flow_classify: convert to new ethdev " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 34/39] examples/flow_filtering: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 35/39] examples/packet_ordering: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 36/39] examples/ptpclient: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 37/39] examples/rxtx_callbacks: " Shahaf Shuler
2017-12-11 16:22 ` Bruce Richardson
2017-11-23 12:19 ` [dpdk-dev] [PATCH 38/39] examples/server_node_efd: " Shahaf Shuler
2017-11-23 12:19 ` [dpdk-dev] [PATCH 39/39] examples/skeleton: " Shahaf Shuler
2017-12-11 16:23 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cc4858f5-d1e5-0326-df11-68cfa81ea64d@intel.com \
--to=radu.nicolau@intel.com \
--cc=dev@dpdk.org \
--cc=shahafs@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).