* Re: mbuf changes
From: Wiles, Keith @ 2016-10-24 16:11 UTC (permalink / raw)
To: Morten Brørup; +Cc: dev@dpdk.org, Olivier Matz
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC359EA8B1@smartserver.smartshare.dk>
> On Oct 24, 2016, at 10:49 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
>
> First of all: Thanks for a great DPDK Userspace 2016!
>
>
>
> Continuing the Userspace discussion about Olivier Matz’s proposed mbuf changes...
>
>
>
> 1.
>
> Stephen Hemminger had a noteworthy general comment about keeping metadata for the NIC in the appropriate section of the mbuf: Metadata generated by the NIC’s RX handler belongs in the first cache line, and metadata required by the NIC’s TX handler belongs in the second cache line. This also means that touching the second cache line on ingress should be avoided if possible; and Bruce Richardson mentioned that for this reason m->next was zeroed on free().
>
>
>
> 2.
>
> There seemed to be consensus that the size of m->refcnt should match the size of m->port because a packet could be duplicated on all physical ports for L3 multicast and L2 flooding.
>
> Furthermore, although a single physical machine (i.e. a single server) with 255 physical ports probably doesn’t exist, it might contain more than 255 virtual machines with a virtual port each, so it makes sense extending these mbuf fields from 8 to 16 bits.
I thought we also talked about removing the m->port from the mbuf as it is not really needed.
>
>
>
> 3.
>
> Someone (Bruce Richardson?) suggested moving m->refcnt and m->port to the second cache line, which then generated questions from the audience about the real life purpose of m->port, and if m->port could be removed from the mbuf structure.
>
>
>
> 4.
>
> I suggested using offset -1 for m->refcnt, so m->refcnt becomes 0 on first allocation. This is based on the assumption that other mbuf fields must be zeroed at alloc()/free() anyway, so zeroing m->refcnt is cheaper than setting it to 1.
>
> Furthermore (regardless of m->refcnt offset), I suggested that it is not required to modify m->refcnt when allocating and freeing the mbuf, thus saving one write operation on both alloc() and free(). However, this assumes that m->refcnt debugging, e.g. underrun detection, is not required.
>
>
>
> 5.
>
> And here’s something new to think about:
>
> m->next already reveals if there are more segments to a packet. Which purpose does m->nb_segs serve that is not already covered by m->next?
>
>
>
>
>
> Med venlig hilsen / kind regards
>
>
>
> Morten Brørup
>
> CTO
>
>
>
>
>
> SmartShare Systems A/S
>
> Tonsbakken 16-18
>
> DK-2740 Skovlunde
>
> Denmark
>
>
>
> Office +45 70 20 00 93
>
> Direct +45 89 93 50 22
>
> Mobile +45 25 40 82 12
>
>
>
> mb@smartsharesystems.com <mailto:mb@smartsharesystems.com>
>
> www.smartsharesystems.com <https://www.smartsharesystems.com/>
>
>
>
Regards,
Keith
^ permalink raw reply
* mbuf changes
From: Morten Brørup @ 2016-10-24 15:49 UTC (permalink / raw)
To: dev; +Cc: Olivier Matz
First of all: Thanks for a great DPDK Userspace 2016!
Continuing the Userspace discussion about Olivier Matz’s proposed mbuf changes...
1.
Stephen Hemminger had a noteworthy general comment about keeping metadata for the NIC in the appropriate section of the mbuf: Metadata generated by the NIC’s RX handler belongs in the first cache line, and metadata required by the NIC’s TX handler belongs in the second cache line. This also means that touching the second cache line on ingress should be avoided if possible; and Bruce Richardson mentioned that for this reason m->next was zeroed on free().
2.
There seemed to be consensus that the size of m->refcnt should match the size of m->port because a packet could be duplicated on all physical ports for L3 multicast and L2 flooding.
Furthermore, although a single physical machine (i.e. a single server) with 255 physical ports probably doesn’t exist, it might contain more than 255 virtual machines with a virtual port each, so it makes sense extending these mbuf fields from 8 to 16 bits.
3.
Someone (Bruce Richardson?) suggested moving m->refcnt and m->port to the second cache line, which then generated questions from the audience about the real life purpose of m->port, and if m->port could be removed from the mbuf structure.
4.
I suggested using offset -1 for m->refcnt, so m->refcnt becomes 0 on first allocation. This is based on the assumption that other mbuf fields must be zeroed at alloc()/free() anyway, so zeroing m->refcnt is cheaper than setting it to 1.
Furthermore (regardless of m->refcnt offset), I suggested that it is not required to modify m->refcnt when allocating and freeing the mbuf, thus saving one write operation on both alloc() and free(). However, this assumes that m->refcnt debugging, e.g. underrun detection, is not required.
5.
And here’s something new to think about:
m->next already reveals if there are more segments to a packet. Which purpose does m->nb_segs serve that is not already covered by m->next?
Med venlig hilsen / kind regards
Morten Brørup
CTO
SmartShare Systems A/S
Tonsbakken 16-18
DK-2740 Skovlunde
Denmark
Office +45 70 20 00 93
Direct +45 89 93 50 22
Mobile +45 25 40 82 12
mb@smartsharesystems.com <mailto:mb@smartsharesystems.com>
www.smartsharesystems.com <https://www.smartsharesystems.com/>
^ permalink raw reply
* Re: [PATCH] Revert "bonding: use existing enslaved device queues"
From: Declan Doherty @ 2016-10-24 15:07 UTC (permalink / raw)
To: Jan Blunck
Cc: Eric Kinzie, Bruce Richardson, Ilya Maximets, dev, Heetae Ahn,
Yuanhan Liu, Bernard Iremonger, stable, Thomas Monjalon
In-Reply-To: <CALe+Z01fKQGVMUMPC=-uFr3aeGGOXcvnRyeF7HB-n6pLU2V_MQ@mail.gmail.com>
On 24/10/16 15:51, Jan Blunck wrote:
> On Mon, Oct 24, 2016 at 7:02 AM, Declan Doherty
> <declan.doherty@intel.com> wrote:
>> On 14/10/16 00:37, Eric Kinzie wrote:
>>>
>>> On Wed Oct 12 16:24:21 +0100 2016, Bruce Richardson wrote:
>>>>
>>>> On Wed, Oct 12, 2016 at 04:24:54PM +0300, Ilya Maximets wrote:
>>>>>
>>>>> On 07.10.2016 05:02, Eric Kinzie wrote:
>>>>>>
>>>>>> On Wed Sep 07 15:28:10 +0300 2016, Ilya Maximets wrote:
>>>>>>>
>>>>>>> This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.
>>>>>>>
>>>>>>> It is necessary to reconfigure all queues every time because
>>>>>>> configuration
>>>>>>> can be changed.
>>>>>>>
>>>>>>> For example, if we're reconfiguring bonding device with new memory
>>>>>>> pool,
>>>>>>> already configured queues will still use the old one. And if the old
>>>>>>> mempool be freed, application likely will panic in attempt to use
>>>>>>> freed mempool.
>>>>>>>
>>>>>>> This happens when we use the bonding device with OVS 2.6 while MTU
>>>>>>> reconfiguration:
>>>>>>>
>>>>>>> PANIC in rte_mempool_get_ops():
>>>>>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)"
>>>>>>> failed
>>>>>>>
>>>>>>> Cc: <stable@dpdk.org>
>>>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>>>>> ---
>>>>>>> drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++--------
>>>>>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>>> index b20a272..eb5b6d1 100644
>>>>>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>>> @@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev
>>>>>>> *bonded_eth_dev,
>>>>>>> struct bond_rx_queue *bd_rx_q;
>>>>>>> struct bond_tx_queue *bd_tx_q;
>>>>>>>
>>>>>>> - uint16_t old_nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
>>>>>>> - uint16_t old_nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
>>>>>>> int errval;
>>>>>>> uint16_t q_id;
>>>>>>>
>>>>>>> @@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev
>>>>>>> *bonded_eth_dev,
>>>>>>> }
>>>>>>>
>>>>>>> /* Setup Rx Queues */
>>>>>>> - /* Use existing queues, if any */
>>>>>>> - for (q_id = old_nb_rx_queues;
>>>>>>> - q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>>>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues;
>>>>>>> q_id++) {
>>>>>>> bd_rx_q = (struct bond_rx_queue
>>>>>>> *)bonded_eth_dev->data->rx_queues[q_id];
>>>>>>>
>>>>>>> errval =
>>>>>>> rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>>>> @@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev
>>>>>>> *bonded_eth_dev,
>>>>>>> }
>>>>>>>
>>>>>>> /* Setup Tx Queues */
>>>>>>> - /* Use existing queues, if any */
>>>>>>> - for (q_id = old_nb_tx_queues;
>>>>>>> - q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>>>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues;
>>>>>>> q_id++) {
>>>>>>> bd_tx_q = (struct bond_tx_queue
>>>>>>> *)bonded_eth_dev->data->tx_queues[q_id];
>>>>>>>
>>>>>>> errval =
>>>>>>> rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>>>> --
>>>>>>> 2.7.4
>>>>>>>
>>>>>>
>>>>>> NAK
>>>>>>
>>>>>> There are still some users of this code. Let's give them a chance to
>>>>>> comment before removing it.
>>>>>
>>>>>
>>>>> Hi Eric,
>>>>>
>>>>> Are these users in CC-list? If not, could you, please, add them?
>>>>> This patch awaits in mail-list already more than a month. I think, it's
>>>>> enough
>>>>> time period for all who wants to say something. Patch fixes a real bug
>>>>> that
>>>>> prevent using of DPDK bonding in all applications that reconfigures
>>>>> devices
>>>>> in runtime including OVS.
>>>>>
>>>> Agreed.
>>>>
>>>> Eric, does reverting this patch cause you problems directly, or is your
>>>> concern
>>>> just with regards to potential impact to others?
>>>>
>>>> Thanks,
>>>> /Bruce
>>>
>>>
>>> This won't impact me directly. The users are CCed (different thread)
>>> and I haven't seen any comment, so I no longer have any objection to
>>> reverting this change.
>>>
>>> Eric
>>>
>>
>> As there has been no further objections and this reinstates the original
>> expected behavior of the bonding driver. I'm re-ack'ing for inclusion in
>> release.
>>
>> Acked-by: Declan Doherty <declan.doherty@intel.com>
>
> Ok, I can revert the revert for us.
>
> Do I read this correctly that you are not interested in fixing this properly?!
>
> Thanks,
> Jan
>
Jan, sorry I missed the replies from last week due to the way my mail
client was filtering the conversation. Let me have another look at this
and I'll come back to the list.
Thanks
Declan
^ permalink raw reply
* Re: [PATCH] Revert "bonding: use existing enslaved device queues"
From: Jan Blunck @ 2016-10-24 14:54 UTC (permalink / raw)
To: Ilya Maximets
Cc: dev, Declan Doherty, Heetae Ahn, Yuanhan Liu, Eric Kinzie,
Bernard Iremonger, stable, Dyasly Sergey
In-Reply-To: <c0fbc18a-95f8-606c-cfcb-70e4055c0391@samsung.com>
On Wed, Oct 19, 2016 at 5:47 AM, Ilya Maximets <i.maximets@samsung.com> wrote:
> On 18.10.2016 18:19, Jan Blunck wrote:
>> On Tue, Oct 18, 2016 at 2:49 PM, Ilya Maximets <i.maximets@samsung.com> wrote:
>>> On 18.10.2016 15:28, Jan Blunck wrote:
>>>> If the application already configured queues the PMD should not
>>>> silently claim ownership and reset them.
>>>>
>>>> What exactly is the problem when changing MTU? This works fine from
>>>> what I can tell.
>>>
>>> Following scenario leads to APP PANIC:
>>>
>>> 1. mempool_1 = rte_mempool_create()
>>> 2. rte_eth_rx_queue_setup(bond0, ..., mempool_1);
>>> 3. rte_eth_dev_start(bond0);
>>> 4. mempool_2 = rte_mempool_create();
>>> 5. rte_eth_dev_stop(bond0);
>>> 6. rte_eth_rx_queue_setup(bond0, ..., mempool_2);
>>> 7. rte_eth_dev_start(bond0);
>>> * RX queues still use 'mempool_1' because reconfiguration doesn't affect them. *
>>> 8. rte_mempool_free(mempool_1);
>>> 9. On any rx operation we'll get PANIC because of using freed 'mempool_1':
>>> PANIC in rte_mempool_get_ops():
>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)" failed
>>>
>>> You may just start OVS 2.6 with DPDK bonding device and attempt to change MTU via 'mtu_request'.
>>> Bug is easily reproducible.
>>>
>>
>> I see. I'm not 100% that this is expected to work without leaking the
>> driver's queues though. The driver is allowed to do allocations in
>> its rx_queue_setup() function that are being freed via
>> rx_queue_release() later. But rx_queue_release() is only called if you
>> reconfigure the
>> device with 0 queues. From what I understand there is no other way to
>> reconfigure a device to use another mempool.
>>
>> But ... even that wouldn't work with the bonding driver right now: the
>> bonding master only configures the slaves during startup. I can put
>> that on my todo list.
>>
>> Coming back to your original problem: changing the MTU for the bond
>> does work through rte_eth_dev_set_mtu() for slaves supporting that. In
>> any other case you could (re-)configure rxmode.max_rx_pkt_len (and
>> jumbo_frame / enable_scatter accordingly). This does work without a
>> call to rte_eth_rx_queue_setup().
>
> Thanks for suggestion, but using of rte_eth_dev_set_mtu() without
> reconfiguration will require to have mempools with huge mbufs (9KB)
> for all ports from the start. This is unacceptable because leads to
> significant performance regressions because of fast cache exhausting.
> Also this will require big work to rewrite OVS reconfiguration code
> this way.
> Anyway, it isn't the MTU only problem. Number of rx/tx descriptors
> also can't be changed in runtime.
>
>
> I'm not fully understand what is the use case for this 'reusing' code.
> Could you, please, describe situation where this behaviour is necessary?
The device that is added to the bond was used before and therefore
already has allocated queues. Therefore we reuse the existing queues
of the devices instead of borrowing the queues of the bond device. If
the slave is removed from the bond again there is no need to allocate
the queues again.
Hope that clarifies the usecase,
Jan
>
> Best regards, Ilya Maximets.
>
>>>
>>>>
>>>> On Wed, Sep 7, 2016 at 2:28 PM, Ilya Maximets <i.maximets@samsung.com> wrote:
>>>>> This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.
>>>>>
>>>>> It is necessary to reconfigure all queues every time because configuration
>>>>> can be changed.
>>>>>
>>>>> For example, if we're reconfiguring bonding device with new memory pool,
>>>>> already configured queues will still use the old one. And if the old
>>>>> mempool be freed, application likely will panic in attempt to use
>>>>> freed mempool.
>>>>>
>>>>> This happens when we use the bonding device with OVS 2.6 while MTU
>>>>> reconfiguration:
>>>>>
>>>>> PANIC in rte_mempool_get_ops():
>>>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)" failed
>>>>>
>>>>> Cc: <stable@dpdk.org>
>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>>> ---
>>>>> drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++--------
>>>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>> index b20a272..eb5b6d1 100644
>>>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>> @@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
>>>>> struct bond_rx_queue *bd_rx_q;
>>>>> struct bond_tx_queue *bd_tx_q;
>>>>>
>>>>> - uint16_t old_nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
>>>>> - uint16_t old_nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
>>>>> int errval;
>>>>> uint16_t q_id;
>>>>>
>>>>> @@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
>>>>> }
>>>>>
>>>>> /* Setup Rx Queues */
>>>>> - /* Use existing queues, if any */
>>>>> - for (q_id = old_nb_rx_queues;
>>>>> - q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>>>>> bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id];
>>>>>
>>>>> errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>> @@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
>>>>> }
>>>>>
>>>>> /* Setup Tx Queues */
>>>>> - /* Use existing queues, if any */
>>>>> - for (q_id = old_nb_tx_queues;
>>>>> - q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>>>>> bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id];
>>>>>
>>>>> errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
^ permalink raw reply
* Re: [PATCH] Revert "bonding: use existing enslaved device queues"
From: Jan Blunck @ 2016-10-24 14:51 UTC (permalink / raw)
To: Declan Doherty
Cc: Eric Kinzie, Bruce Richardson, Ilya Maximets, dev, Heetae Ahn,
Yuanhan Liu, Bernard Iremonger, stable, Thomas Monjalon
In-Reply-To: <61df7d78-c57a-d379-252a-aa7128e7e62e@intel.com>
On Mon, Oct 24, 2016 at 7:02 AM, Declan Doherty
<declan.doherty@intel.com> wrote:
> On 14/10/16 00:37, Eric Kinzie wrote:
>>
>> On Wed Oct 12 16:24:21 +0100 2016, Bruce Richardson wrote:
>>>
>>> On Wed, Oct 12, 2016 at 04:24:54PM +0300, Ilya Maximets wrote:
>>>>
>>>> On 07.10.2016 05:02, Eric Kinzie wrote:
>>>>>
>>>>> On Wed Sep 07 15:28:10 +0300 2016, Ilya Maximets wrote:
>>>>>>
>>>>>> This reverts commit 5b7bb2bda5519b7800f814df64d4e015282140e5.
>>>>>>
>>>>>> It is necessary to reconfigure all queues every time because
>>>>>> configuration
>>>>>> can be changed.
>>>>>>
>>>>>> For example, if we're reconfiguring bonding device with new memory
>>>>>> pool,
>>>>>> already configured queues will still use the old one. And if the old
>>>>>> mempool be freed, application likely will panic in attempt to use
>>>>>> freed mempool.
>>>>>>
>>>>>> This happens when we use the bonding device with OVS 2.6 while MTU
>>>>>> reconfiguration:
>>>>>>
>>>>>> PANIC in rte_mempool_get_ops():
>>>>>> assert "(ops_index >= 0) && (ops_index < RTE_MEMPOOL_MAX_OPS_IDX)"
>>>>>> failed
>>>>>>
>>>>>> Cc: <stable@dpdk.org>
>>>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>>>> ---
>>>>>> drivers/net/bonding/rte_eth_bond_pmd.c | 10 ++--------
>>>>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>> index b20a272..eb5b6d1 100644
>>>>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>>>>>> @@ -1305,8 +1305,6 @@ slave_configure(struct rte_eth_dev
>>>>>> *bonded_eth_dev,
>>>>>> struct bond_rx_queue *bd_rx_q;
>>>>>> struct bond_tx_queue *bd_tx_q;
>>>>>>
>>>>>> - uint16_t old_nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
>>>>>> - uint16_t old_nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
>>>>>> int errval;
>>>>>> uint16_t q_id;
>>>>>>
>>>>>> @@ -1347,9 +1345,7 @@ slave_configure(struct rte_eth_dev
>>>>>> *bonded_eth_dev,
>>>>>> }
>>>>>>
>>>>>> /* Setup Rx Queues */
>>>>>> - /* Use existing queues, if any */
>>>>>> - for (q_id = old_nb_rx_queues;
>>>>>> - q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues;
>>>>>> q_id++) {
>>>>>> bd_rx_q = (struct bond_rx_queue
>>>>>> *)bonded_eth_dev->data->rx_queues[q_id];
>>>>>>
>>>>>> errval =
>>>>>> rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>>> @@ -1365,9 +1361,7 @@ slave_configure(struct rte_eth_dev
>>>>>> *bonded_eth_dev,
>>>>>> }
>>>>>>
>>>>>> /* Setup Tx Queues */
>>>>>> - /* Use existing queues, if any */
>>>>>> - for (q_id = old_nb_tx_queues;
>>>>>> - q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>>>>>> + for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues;
>>>>>> q_id++) {
>>>>>> bd_tx_q = (struct bond_tx_queue
>>>>>> *)bonded_eth_dev->data->tx_queues[q_id];
>>>>>>
>>>>>> errval =
>>>>>> rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>
>>>>> NAK
>>>>>
>>>>> There are still some users of this code. Let's give them a chance to
>>>>> comment before removing it.
>>>>
>>>>
>>>> Hi Eric,
>>>>
>>>> Are these users in CC-list? If not, could you, please, add them?
>>>> This patch awaits in mail-list already more than a month. I think, it's
>>>> enough
>>>> time period for all who wants to say something. Patch fixes a real bug
>>>> that
>>>> prevent using of DPDK bonding in all applications that reconfigures
>>>> devices
>>>> in runtime including OVS.
>>>>
>>> Agreed.
>>>
>>> Eric, does reverting this patch cause you problems directly, or is your
>>> concern
>>> just with regards to potential impact to others?
>>>
>>> Thanks,
>>> /Bruce
>>
>>
>> This won't impact me directly. The users are CCed (different thread)
>> and I haven't seen any comment, so I no longer have any objection to
>> reverting this change.
>>
>> Eric
>>
>
> As there has been no further objections and this reinstates the original
> expected behavior of the bonding driver. I'm re-ack'ing for inclusion in
> release.
>
> Acked-by: Declan Doherty <declan.doherty@intel.com>
Ok, I can revert the revert for us.
Do I read this correctly that you are not interested in fixing this properly?!
Thanks,
Jan
^ permalink raw reply
* Re: [PATCH v2 0/2] net/ixgbe: fix VF VLAN insert
From: Bruce Richardson @ 2016-10-24 14:19 UTC (permalink / raw)
To: Lu, Wenzhuo
Cc: Iremonger, Bernard, dev@dpdk.org, daniels@research.att.com,
az5157@att.com
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093933D3DC@shsmsx102.ccr.corp.intel.com>
On Thu, Oct 20, 2016 at 12:54:52AM +0000, Lu, Wenzhuo wrote:
> Hi,
>
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Wednesday, October 19, 2016 10:48 PM
> > To: dev@dpdk.org; daniels@research.att.com; Lu, Wenzhuo; az5157@att.com
> > Cc: Iremonger, Bernard
> > Subject: [PATCH v2 0/2] net/ixgbe: fix VF VLAN insert
> >
> > Changes in v2:
> > Add testpmd patch.
> > Update testpmd for change to rte_pmd_ixgbe_set_vf_vlan_insert function.
> >
> > Bernard Iremonger (1):
> > app/test_pmd: change to the VF VLAN insert command
> >
> > E. Scott Daniels (1):
> > net/ixgbe: fix VLAN insert parameter type and its use
> >
> > app/test-pmd/cmdline.c | 19 +++++++++----------
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
> > drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++++----
> > drivers/net/ixgbe/rte_pmd_ixgbe.h | 9 +++++----
> > 4 files changed, 19 insertions(+), 19 deletions(-)
> >
> > --
> > 2.10.1
> Series-Acked-by: Wenzhuo Lu <Wenzhuo.lu@intel.com>
>
Applied to dpdk-next-net/rel_16_11
/Bruce
^ permalink raw reply
* [PATCH v9 6/6] testpmd: use Tx preparation in csum engine
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Removed pseudo header calculation for udp/tcp/tso packets from
application and used Tx preparation API for packet preparation and
verification.
Adding additional step to the csum engine costs about 3-4% of performance
drop, on my setup with ixgbe driver. It's caused mostly by the need
of reaccessing and modification of packet data.
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
app/test-pmd/csumonly.c | 36 +++++++++++++-----------------------
1 file changed, 13 insertions(+), 23 deletions(-)
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 57e6ae2..6f33ae9 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -112,15 +112,6 @@ struct simple_gre_hdr {
} __attribute__((__packed__));
static uint16_t
-get_psd_sum(void *l3_hdr, uint16_t ethertype, uint64_t ol_flags)
-{
- if (ethertype == _htons(ETHER_TYPE_IPv4))
- return rte_ipv4_phdr_cksum(l3_hdr, ol_flags);
- else /* assume ethertype == ETHER_TYPE_IPv6 */
- return rte_ipv6_phdr_cksum(l3_hdr, ol_flags);
-}
-
-static uint16_t
get_udptcp_checksum(void *l3_hdr, void *l4_hdr, uint16_t ethertype)
{
if (ethertype == _htons(ETHER_TYPE_IPv4))
@@ -370,32 +361,24 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
/* do not recalculate udp cksum if it was 0 */
if (udp_hdr->dgram_cksum != 0) {
udp_hdr->dgram_cksum = 0;
- if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_UDP_CKSUM) {
+ if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_UDP_CKSUM)
ol_flags |= PKT_TX_UDP_CKSUM;
- udp_hdr->dgram_cksum = get_psd_sum(l3_hdr,
- info->ethertype, ol_flags);
- } else {
+ else
udp_hdr->dgram_cksum =
get_udptcp_checksum(l3_hdr, udp_hdr,
info->ethertype);
- }
}
} else if (info->l4_proto == IPPROTO_TCP) {
tcp_hdr = (struct tcp_hdr *)((char *)l3_hdr + info->l3_len);
tcp_hdr->cksum = 0;
- if (tso_segsz) {
+ if (tso_segsz)
ol_flags |= PKT_TX_TCP_SEG;
- tcp_hdr->cksum = get_psd_sum(l3_hdr, info->ethertype,
- ol_flags);
- } else if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_TCP_CKSUM) {
+ else if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_TCP_CKSUM)
ol_flags |= PKT_TX_TCP_CKSUM;
- tcp_hdr->cksum = get_psd_sum(l3_hdr, info->ethertype,
- ol_flags);
- } else {
+ else
tcp_hdr->cksum =
get_udptcp_checksum(l3_hdr, tcp_hdr,
info->ethertype);
- }
} else if (info->l4_proto == IPPROTO_SCTP) {
sctp_hdr = (struct sctp_hdr *)((char *)l3_hdr + info->l3_len);
sctp_hdr->cksum = 0;
@@ -648,6 +631,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
void *l3_hdr = NULL, *outer_l3_hdr = NULL; /* can be IPv4 or IPv6 */
uint16_t nb_rx;
uint16_t nb_tx;
+ uint16_t nb_prep;
uint16_t i;
uint64_t rx_ol_flags, tx_ol_flags;
uint16_t testpmd_ol_flags;
@@ -857,7 +841,13 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
printf("\n");
}
}
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
+ nb_prep = rte_eth_tx_prep(fs->tx_port, fs->tx_queue, pkts_burst,
+ nb_rx);
+ if (nb_prep != nb_rx)
+ printf("Preparing packet burst to transmit failed: %s\n",
+ rte_strerror(rte_errno));
+
+ nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_prep);
/*
* Retry if necessary
*/
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 5/6] ixgbe: add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++
drivers/net/ixgbe/ixgbe_ethdev.h | 5 +++-
drivers/net/ixgbe/ixgbe_rxtx.c | 58 +++++++++++++++++++++++++++++++++++++-
drivers/net/ixgbe/ixgbe_rxtx.h | 2 ++
4 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 4ca5747..4c6a8e1 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -517,6 +517,8 @@ static const struct rte_eth_desc_lim tx_desc_lim = {
.nb_max = IXGBE_MAX_RING_DESC,
.nb_min = IXGBE_MIN_RING_DESC,
.nb_align = IXGBE_TXD_ALIGN,
+ .nb_seg_max = IXGBE_TX_MAX_SEG,
+ .nb_mtu_seg_max = IXGBE_TX_MAX_SEG,
};
static const struct eth_dev_ops ixgbe_eth_dev_ops = {
@@ -1103,6 +1105,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->dev_ops = &ixgbe_eth_dev_ops;
eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts;
+ eth_dev->tx_pkt_prep = &ixgbe_prep_pkts;
/*
* For secondary processes, we don't initialise any further as primary
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 4ff6338..e229cf5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -396,6 +396,9 @@ uint16_t ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t ixgbe_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t ixgbe_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+
int ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
struct rte_eth_rss_conf *rss_conf);
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2ce8234..031414c 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* Copyright 2014 6WIND S.A.
* All rights reserved.
*
@@ -70,6 +70,7 @@
#include <rte_string_fns.h>
#include <rte_errno.h>
#include <rte_ip.h>
+#include <rte_net.h>
#include "ixgbe_logs.h"
#include "base/ixgbe_api.h"
@@ -87,6 +88,9 @@
PKT_TX_TCP_SEG | \
PKT_TX_OUTER_IP_CKSUM)
+#define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ IXGBE_TX_OFFLOAD_MASK)
+
#if 1
#define RTE_PMD_USE_PREFETCH
#endif
@@ -905,6 +909,56 @@ end_of_tx:
/*********************************************************************
*
+ * TX prep functions
+ *
+ **********************************************************************/
+uint16_t
+ixgbe_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
+{
+ int i, ret;
+ uint64_t ol_flags;
+ struct rte_mbuf *m;
+ struct ixgbe_tx_queue *txq = (struct ixgbe_tx_queue *)tx_queue;
+
+ for (i = 0; i < nb_pkts; i++) {
+ m = tx_pkts[i];
+ ol_flags = m->ol_flags;
+
+ /**
+ * Check if packet meets requirements for number of segments
+ *
+ * NOTE: for ixgbe it's always (40 - WTHRESH) for both TSO and non-TSO
+ */
+
+ if (m->nb_segs > IXGBE_TX_MAX_SEG - txq->wthresh) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+ if (ol_flags & IXGBE_TX_OFFLOAD_NOTSUP_MASK) {
+ rte_errno = -ENOTSUP;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_phdr_cksum_fix(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+
+ return i;
+}
+
+/*********************************************************************
+ *
* RX functions
*
**********************************************************************/
@@ -2282,6 +2336,7 @@ ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq)
if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) == IXGBE_SIMPLE_FLAGS)
&& (txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST)) {
PMD_INIT_LOG(DEBUG, "Using simple tx code path");
+ dev->tx_pkt_prep = NULL;
#ifdef RTE_IXGBE_INC_VECTOR
if (txq->tx_rs_thresh <= RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&
(rte_eal_process_type() != RTE_PROC_PRIMARY ||
@@ -2302,6 +2357,7 @@ ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq)
(unsigned long)txq->tx_rs_thresh,
(unsigned long)RTE_PMD_IXGBE_TX_MAX_BURST);
dev->tx_pkt_burst = ixgbe_xmit_pkts;
+ dev->tx_pkt_prep = ixgbe_prep_pkts;
}
}
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 2608b36..7bbd9b8 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -80,6 +80,8 @@
#define RTE_IXGBE_WAIT_100_US 100
#define RTE_IXGBE_VMTXSW_REGISTER_COUNT 2
+#define IXGBE_TX_MAX_SEG 40
+
#define IXGBE_PACKET_TYPE_MASK_82599 0X7F
#define IXGBE_PACKET_TYPE_MASK_X550 0X10FF
#define IXGBE_PACKET_TYPE_MASK_TUNNEL 0XFF
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 4/6] i40e: add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 3 ++
drivers/net/i40e/i40e_rxtx.c | 72 +++++++++++++++++++++++++++++++++++++++-
drivers/net/i40e/i40e_rxtx.h | 8 +++++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5af0e43..dab0d48 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -936,6 +936,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
dev->dev_ops = &i40e_eth_dev_ops;
dev->rx_pkt_burst = i40e_recv_pkts;
dev->tx_pkt_burst = i40e_xmit_pkts;
+ dev->tx_pkt_prep = i40e_prep_pkts;
/* for secondary processes, we don't initialise any further as primary
* has already done this work. Only check we don't need a different
@@ -2629,6 +2630,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
.nb_max = I40E_MAX_RING_DESC,
.nb_min = I40E_MIN_RING_DESC,
.nb_align = I40E_ALIGN_RING_DESC,
+ .nb_seg_max = I40E_TX_MAX_SEG,
+ .nb_mtu_seg_max = I40E_TX_MAX_MTU_SEG,
};
if (pf->flags & I40E_FLAG_VMDQ) {
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 7ae7d9f..7f6d3d8 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -50,6 +50,8 @@
#include <rte_tcp.h>
#include <rte_sctp.h>
#include <rte_udp.h>
+#include <rte_ip.h>
+#include <rte_net.h>
#include "i40e_logs.h"
#include "base/i40e_prototype.h"
@@ -79,6 +81,17 @@
PKT_TX_TCP_SEG | \
PKT_TX_OUTER_IP_CKSUM)
+#define I40E_TX_OFFLOAD_MASK ( \
+ PKT_TX_IP_CKSUM | \
+ PKT_TX_L4_MASK | \
+ PKT_TX_OUTER_IP_CKSUM | \
+ PKT_TX_TCP_SEG | \
+ PKT_TX_QINQ_PKT | \
+ PKT_TX_VLAN_PKT)
+
+#define I40E_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ I40E_TX_OFFLOAD_MASK)
+
static uint16_t i40e_xmit_pkts_simple(void *tx_queue,
struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
@@ -1411,6 +1424,61 @@ i40e_xmit_pkts_simple(void *tx_queue,
return nb_tx;
}
+/*********************************************************************
+ *
+ * TX prep functions
+ *
+ **********************************************************************/
+uint16_t
+i40e_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ int i, ret;
+ uint64_t ol_flags;
+ struct rte_mbuf *m;
+
+ for (i = 0; i < nb_pkts; i++) {
+ m = tx_pkts[i];
+ ol_flags = m->ol_flags;
+
+ /**
+ * m->nb_segs is uint8_t, so m->nb_segs is always less than
+ * I40E_TX_MAX_SEG.
+ * We check only a condition for m->nb_segs > I40E_TX_MAX_MTU_SEG.
+ */
+ if (!(ol_flags & PKT_TX_TCP_SEG)) {
+ if (m->nb_segs > I40E_TX_MAX_MTU_SEG) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+ } else if ((m->tso_segsz < I40E_MIN_TSO_MSS) ||
+ (m->tso_segsz > I40E_MAX_TSO_MSS)) {
+ /* MSS outside the range (256B - 9674B) are considered malicious */
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+ if (ol_flags & I40E_TX_OFFLOAD_NOTSUP_MASK) {
+ rte_errno = -ENOTSUP;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_phdr_cksum_fix(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+ return i;
+}
+
/*
* Find the VSI the queue belongs to. 'queue_idx' is the queue index
* application used, which assume having sequential ones. But from driver's
@@ -2763,9 +2831,11 @@ i40e_set_tx_function(struct rte_eth_dev *dev)
PMD_INIT_LOG(DEBUG, "Simple tx finally be used.");
dev->tx_pkt_burst = i40e_xmit_pkts_simple;
}
+ dev->tx_pkt_prep = NULL;
} else {
PMD_INIT_LOG(DEBUG, "Xmit tx finally be used.");
dev->tx_pkt_burst = i40e_xmit_pkts;
+ dev->tx_pkt_prep = i40e_prep_pkts;
}
}
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index ecdb13c..9df8a56 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -63,6 +63,12 @@
#define I40E_MIN_RING_DESC 64
#define I40E_MAX_RING_DESC 4096
+#define I40E_MIN_TSO_MSS 256
+#define I40E_MAX_TSO_MSS 9674
+
+#define I40E_TX_MAX_SEG UINT8_MAX
+#define I40E_TX_MAX_MTU_SEG 8
+
#undef container_of
#define container_of(ptr, type, member) ({ \
typeof(((type *)0)->member)(*__mptr) = (ptr); \
@@ -223,6 +229,8 @@ uint16_t i40e_recv_scattered_pkts(void *rx_queue,
uint16_t i40e_xmit_pkts(void *tx_queue,
struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t i40e_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
int i40e_tx_queue_init(struct i40e_tx_queue *txq);
int i40e_rx_queue_init(struct i40e_rx_queue *rxq);
void i40e_free_tx_resources(struct i40e_tx_queue *txq);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 3/6] fm10k: add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
drivers/net/fm10k/fm10k.h | 6 +++++
drivers/net/fm10k/fm10k_ethdev.c | 5 ++++
drivers/net/fm10k/fm10k_rxtx.c | 50 +++++++++++++++++++++++++++++++++++++-
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
index 05aa1a2..c6fed21 100644
--- a/drivers/net/fm10k/fm10k.h
+++ b/drivers/net/fm10k/fm10k.h
@@ -69,6 +69,9 @@
#define FM10K_MAX_RX_DESC (FM10K_MAX_RX_RING_SZ / sizeof(union fm10k_rx_desc))
#define FM10K_MAX_TX_DESC (FM10K_MAX_TX_RING_SZ / sizeof(struct fm10k_tx_desc))
+#define FM10K_TX_MAX_SEG UINT8_MAX
+#define FM10K_TX_MAX_MTU_SEG UINT8_MAX
+
/*
* byte aligment for HW RX data buffer
* Datasheet requires RX buffer addresses shall either be 512-byte aligned or
@@ -356,6 +359,9 @@ fm10k_dev_rx_descriptor_done(void *rx_queue, uint16_t offset);
uint16_t fm10k_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t fm10k_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+
int fm10k_rxq_vec_setup(struct fm10k_rx_queue *rxq);
int fm10k_rx_vec_condition_check(struct rte_eth_dev *);
void fm10k_rx_queue_release_mbufs_vec(struct fm10k_rx_queue *rxq);
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index c804436..dffb6d1 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1446,6 +1446,8 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
.nb_max = FM10K_MAX_TX_DESC,
.nb_min = FM10K_MIN_TX_DESC,
.nb_align = FM10K_MULT_TX_DESC,
+ .nb_seg_max = FM10K_TX_MAX_SEG,
+ .nb_mtu_seg_max = FM10K_TX_MAX_MTU_SEG,
};
dev_info->speed_capa = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_2_5G |
@@ -2754,8 +2756,10 @@ fm10k_set_tx_function(struct rte_eth_dev *dev)
fm10k_txq_vec_setup(txq);
}
dev->tx_pkt_burst = fm10k_xmit_pkts_vec;
+ dev->tx_pkt_prep = NULL;
} else {
dev->tx_pkt_burst = fm10k_xmit_pkts;
+ dev->tx_pkt_prep = fm10k_prep_pkts;
PMD_INIT_LOG(DEBUG, "Use regular Tx func");
}
}
@@ -2834,6 +2838,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
dev->dev_ops = &fm10k_eth_dev_ops;
dev->rx_pkt_burst = &fm10k_recv_pkts;
dev->tx_pkt_burst = &fm10k_xmit_pkts;
+ dev->tx_pkt_prep = &fm10k_prep_pkts;
/* only initialize in the primary process */
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 32cc7ff..5fc4d5a 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2013-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2013-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -35,6 +35,7 @@
#include <rte_ethdev.h>
#include <rte_common.h>
+#include <rte_net.h>
#include "fm10k.h"
#include "base/fm10k_type.h"
@@ -65,6 +66,15 @@ static inline void dump_rxd(union fm10k_rx_desc *rxd)
}
#endif
+#define FM10K_TX_OFFLOAD_MASK ( \
+ PKT_TX_VLAN_PKT | \
+ PKT_TX_IP_CKSUM | \
+ PKT_TX_L4_MASK | \
+ PKT_TX_TCP_SEG)
+
+#define FM10K_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ FM10K_TX_OFFLOAD_MASK)
+
/* @note: When this function is changed, make corresponding change to
* fm10k_dev_supported_ptypes_get()
*/
@@ -597,3 +607,41 @@ fm10k_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
return count;
}
+
+uint16_t
+fm10k_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ int i, ret;
+ struct rte_mbuf *m;
+
+ for (i = 0; i < nb_pkts; i++) {
+ m = tx_pkts[i];
+
+ if ((m->ol_flags & PKT_TX_TCP_SEG) &&
+ (m->tso_segsz < FM10K_TSO_MINMSS)) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+ if (m->ol_flags & FM10K_TX_OFFLOAD_NOTSUP_MASK) {
+ rte_errno = -ENOTSUP;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_phdr_cksum_fix(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+
+ return i;
+}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v8 1/6] ethdev: add Tx preparation
From: Kulasek, TomaszX @ 2016-10-24 14:12 UTC (permalink / raw)
To: Ananyev, Konstantin, dev@dpdk.org; +Cc: olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CB102@irsmsx105.ger.corp.intel.com>
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Monday, October 24, 2016 14:57
> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com
> Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
>
>
>
> > -----Original Message-----
> > From: Kulasek, TomaszX
> > Sent: Monday, October 24, 2016 1:49 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> > Cc: olivier.matz@6wind.com
> > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
> >
> > Hi Konstantin,
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Monday, October 24, 2016 14:15
> > > To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> > > Cc: olivier.matz@6wind.com
> > > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
> > >
> > > Hi Tomasz,
> > >
> >
> > [...]
> >
> > > >
> > > > +/**
> > > > + * Fix pseudo header checksum
> > > > + *
> > > > + * This function fixes pseudo header checksum for TSO and non-TSO
> > > > +tcp/udp in
> > > > + * provided mbufs packet data.
> > > > + *
> > > > + * - for non-TSO tcp/udp packets full pseudo-header checksum is
> > > > +counted
> > > and set
> > > > + * in packet data,
> > > > + * - for TSO the IP payload length is not included in pseudo
> header.
> > > > + *
> > > > + * This function expects that used headers are in the first data
> > > > +segment of
> > > > + * mbuf, and are not fragmented.
> > > > + *
> > > > + * @param m
> > > > + * The packet mbuf to be validated.
> > > > + * @return
> > > > + * 0 if checksum is initialized properly
> > > > + */
> > > > +static inline int
> > > > +rte_phdr_cksum_fix(struct rte_mbuf *m) {
> > > > + struct ipv4_hdr *ipv4_hdr;
> > > > + struct ipv6_hdr *ipv6_hdr;
> > > > + struct tcp_hdr *tcp_hdr;
> > > > + struct udp_hdr *udp_hdr;
> > > > + uint64_t ol_flags = m->ol_flags;
> > > > + uint64_t inner_l3_offset = m->l2_len;
> > > > +
> > > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> > > > + inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> > > > +
> > > > + /* headers are fragmented */
> > > > + if (unlikely(rte_pktmbuf_data_len(m) >= inner_l3_offset +
> > > > +m->l3_len
> > > +
> > > > + m->l4_len))
> > >
> > > Might be better to move that check into rte_validate_tx_offload(),
> > > so it would be called only when TX_DEBUG is on.
> >
> > While unfragmented headers are not general requirements for Tx
> > offloads, and this requirement is for this particular implementation,
> maybe for performance reasons will be better to keep it here, and just add
> #if DEBUG to leave rte_validate_tx_offload more generic.
>
> Hmm and what is the advantage to pollute that code with more ifdefs?
> Again, why unfragmented headers are not general requirements?
> As long as DPDK pseudo-headear csum calculation routines can't handle
> fragmented case, it pretty much is a general requirement, no?
> Konstantin
>
Ok, you're right, if we assume that this is general requirement, it should be moved.
> >
> > > Another thing, shouldn't it be:
> > > if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
> ?
> >
> > Yes, it should.
> >
> > > Konstantin
> > >
> >
> > Tomasz
^ permalink raw reply
* [PATCH v9 2/6] e1000: add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
drivers/net/e1000/e1000_ethdev.h | 11 ++++++++
drivers/net/e1000/em_ethdev.c | 5 +++-
drivers/net/e1000/em_rxtx.c | 48 ++++++++++++++++++++++++++++++++++-
drivers/net/e1000/igb_ethdev.c | 4 +++
drivers/net/e1000/igb_rxtx.c | 52 +++++++++++++++++++++++++++++++++++++-
5 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6c25c8d..bd0f277 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -138,6 +138,11 @@
#define E1000_MISC_VEC_ID RTE_INTR_VEC_ZERO_OFFSET
#define E1000_RX_VEC_START RTE_INTR_VEC_RXTX_OFFSET
+#define IGB_TX_MAX_SEG UINT8_MAX
+#define IGB_TX_MAX_MTU_SEG UINT8_MAX
+#define EM_TX_MAX_SEG UINT8_MAX
+#define EM_TX_MAX_MTU_SEG UINT8_MAX
+
/* structure for interrupt relative data */
struct e1000_interrupt {
uint32_t flags;
@@ -315,6 +320,9 @@ void eth_igb_tx_init(struct rte_eth_dev *dev);
uint16_t eth_igb_xmit_pkts(void *txq, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t eth_igb_prep_pkts(void *txq, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+
uint16_t eth_igb_recv_pkts(void *rxq, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
@@ -376,6 +384,9 @@ void eth_em_tx_init(struct rte_eth_dev *dev);
uint16_t eth_em_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts);
+uint16_t eth_em_prep_pkts(void *txq, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+
uint16_t eth_em_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 7cf5f0c..17b45cb 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -300,6 +300,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->dev_ops = ð_em_ops;
eth_dev->rx_pkt_burst = (eth_rx_burst_t)ð_em_recv_pkts;
eth_dev->tx_pkt_burst = (eth_tx_burst_t)ð_em_xmit_pkts;
+ eth_dev->tx_pkt_prep = (eth_tx_prep_t)ð_em_prep_pkts;
/* for secondary processes, we don't initialise any further as primary
* has already done this work. Only check we don't need a different
@@ -1067,6 +1068,8 @@ eth_em_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
.nb_max = E1000_MAX_RING_DESC,
.nb_min = E1000_MIN_RING_DESC,
.nb_align = EM_TXD_ALIGN,
+ .nb_seg_max = EM_TX_MAX_SEG,
+ .nb_mtu_seg_max = EM_TX_MAX_MTU_SEG,
};
dev_info->speed_capa = ETH_LINK_SPEED_10M_HD | ETH_LINK_SPEED_10M |
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 41f51c0..5bd3c99 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -66,6 +66,7 @@
#include <rte_udp.h>
#include <rte_tcp.h>
#include <rte_sctp.h>
+#include <rte_net.h>
#include <rte_string_fns.h>
#include "e1000_logs.h"
@@ -77,6 +78,14 @@
#define E1000_RXDCTL_GRAN 0x01000000 /* RXDCTL Granularity */
+#define E1000_TX_OFFLOAD_MASK ( \
+ PKT_TX_IP_CKSUM | \
+ PKT_TX_L4_MASK | \
+ PKT_TX_VLAN_PKT)
+
+#define E1000_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ E1000_TX_OFFLOAD_MASK)
+
/**
* Structure associated with each descriptor of the RX ring of a RX queue.
*/
@@ -618,6 +627,43 @@ end_of_tx:
/*********************************************************************
*
+ * TX prep functions
+ *
+ **********************************************************************/
+uint16_t
+eth_em_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ int i, ret;
+ struct rte_mbuf *m;
+
+ for (i = 0; i < nb_pkts; i++) {
+ m = tx_pkts[i];
+
+ if (m->ol_flags & E1000_TX_OFFLOAD_NOTSUP_MASK) {
+ rte_errno = -ENOTSUP;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_phdr_cksum_fix(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+
+ return i;
+}
+
+/*********************************************************************
+ *
* RX functions
*
**********************************************************************/
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 4924396..0afdd09 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -369,6 +369,8 @@ static const struct rte_eth_desc_lim tx_desc_lim = {
.nb_max = E1000_MAX_RING_DESC,
.nb_min = E1000_MIN_RING_DESC,
.nb_align = IGB_RXD_ALIGN,
+ .nb_seg_max = IGB_TX_MAX_SEG,
+ .nb_mtu_seg_max = IGB_TX_MAX_MTU_SEG,
};
static const struct eth_dev_ops eth_igb_ops = {
@@ -760,6 +762,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->dev_ops = ð_igb_ops;
eth_dev->rx_pkt_burst = ð_igb_recv_pkts;
eth_dev->tx_pkt_burst = ð_igb_xmit_pkts;
+ eth_dev->tx_pkt_prep = ð_igb_prep_pkts;
/* for secondary processes, we don't initialise any further as primary
* has already done this work. Only check we don't need a different
@@ -963,6 +966,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->dev_ops = &igbvf_eth_dev_ops;
eth_dev->rx_pkt_burst = ð_igb_recv_pkts;
eth_dev->tx_pkt_burst = ð_igb_xmit_pkts;
+ eth_dev->tx_pkt_prep = ð_igb_prep_pkts;
/* for secondary processes, we don't initialise any further as primary
* has already done this work. Only check we don't need a different
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index dbd37ac..08e47f2 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1,7 +1,7 @@
/*-
* BSD LICENSE
*
- * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ * Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -65,6 +65,7 @@
#include <rte_udp.h>
#include <rte_tcp.h>
#include <rte_sctp.h>
+#include <rte_net.h>
#include <rte_string_fns.h>
#include "e1000_logs.h"
@@ -78,6 +79,9 @@
PKT_TX_L4_MASK | \
PKT_TX_TCP_SEG)
+#define IGB_TX_OFFLOAD_NOTSUP_MASK \
+ (PKT_TX_OFFLOAD_MASK ^ IGB_TX_OFFLOAD_MASK)
+
/**
* Structure associated with each descriptor of the RX ring of a RX queue.
*/
@@ -616,6 +620,51 @@ eth_igb_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
/*********************************************************************
*
+ * TX prep functions
+ *
+ **********************************************************************/
+uint16_t
+eth_igb_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ int i, ret;
+ struct rte_mbuf *m;
+
+ for (i = 0; i < nb_pkts; i++) {
+ m = tx_pkts[i];
+
+ /* Check some limitations for TSO in hardware */
+ if (m->ol_flags & PKT_TX_TCP_SEG)
+ if ((m->tso_segsz > IGB_TSO_MAX_MSS) || (m->l2_len + m->l3_len +
+ m->l4_len > IGB_TSO_MAX_HDRLEN)) {
+ rte_errno = -EINVAL;
+ return i;
+ }
+
+ if (m->ol_flags & IGB_TX_OFFLOAD_NOTSUP_MASK) {
+ rte_errno = -ENOTSUP;
+ return i;
+ }
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ ret = rte_validate_tx_offload(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+#endif
+ ret = rte_phdr_cksum_fix(m);
+ if (ret != 0) {
+ rte_errno = ret;
+ return i;
+ }
+ }
+
+ return i;
+}
+
+/*********************************************************************
+ *
* RX functions
*
**********************************************************************/
@@ -1364,6 +1413,7 @@ eth_igb_tx_queue_setup(struct rte_eth_dev *dev,
igb_reset_tx_queue(txq, dev);
dev->tx_pkt_burst = eth_igb_xmit_pkts;
+ dev->tx_pkt_prep = ð_igb_prep_pkts;
dev->data->tx_queues[queue_idx] = txq;
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 1/6] ethdev: add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477317933-14144-1-git-send-email-tomaszx.kulasek@intel.com>
Added API for `rte_eth_tx_prep`
uint16_t rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id,
struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
Added fields to the `struct rte_eth_desc_lim`:
uint16_t nb_seg_max;
/**< Max number of segments per whole packet. */
uint16_t nb_mtu_seg_max;
/**< Max number of segments per one MTU */
Added functions:
int rte_validate_tx_offload(struct rte_mbuf *m)
to validate general requirements for tx offload set in mbuf of packet
such a flag completness. In current implementation this function is
called optionaly when RTE_LIBRTE_ETHDEV_DEBUG is enabled.
int rte_phdr_cksum_fix(struct rte_mbuf *m)
to fix pseudo header checksum for TSO and non-TSO tcp/udp packets
before hardware tx checksum offload.
- for non-TSO tcp/udp packets full pseudo-header checksum is
counted and set.
- for TSO the IP payload length is not included.
PERFORMANCE TESTS
-----------------
This feature was tested with modified csum engine from test-pmd.
The packet checksum preparation was moved from application to Tx
preparation step placed before burst.
We may expect some overhead costs caused by:
1) using additional callback before burst,
2) rescanning burst,
3) additional condition checking (packet validation),
4) worse optimization (e.g. packet data access, etc.)
We tested it using ixgbe Tx preparation implementation with some parts
disabled to have comparable information about the impact of different
parts of implementation.
IMPACT:
1) For unimplemented Tx preparation callback the performance impact is
negligible,
2) For packet condition check without checksum modifications (nb_segs,
available offloads, etc.) is 14626628/14252168 (~2.62% drop),
3) Full support in ixgbe driver (point 2 + packet checksum
initialization) is 14060924/13588094 (~3.48% drop)
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
config/common_base | 1 +
lib/librte_ether/rte_ethdev.h | 97 +++++++++++++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf.h | 64 +++++++++++++++++++++++++++
lib/librte_net/rte_net.h | 85 ++++++++++++++++++++++++++++++++++++
4 files changed, 247 insertions(+)
diff --git a/config/common_base b/config/common_base
index c7fd3db..619284b 100644
--- a/config/common_base
+++ b/config/common_base
@@ -120,6 +120,7 @@ CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
CONFIG_RTE_LIBRTE_IEEE1588=n
CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
+CONFIG_RTE_ETHDEV_TX_PREP=y
#
# Support NIC bypass logic
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 38641e8..d548d48 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -182,6 +182,7 @@ extern "C" {
#include <rte_pci.h>
#include <rte_dev.h>
#include <rte_devargs.h>
+#include <rte_errno.h>
#include "rte_ether.h"
#include "rte_eth_ctrl.h"
#include "rte_dev_info.h"
@@ -699,6 +700,8 @@ struct rte_eth_desc_lim {
uint16_t nb_max; /**< Max allowed number of descriptors. */
uint16_t nb_min; /**< Min allowed number of descriptors. */
uint16_t nb_align; /**< Number of descriptors should be aligned to. */
+ uint16_t nb_seg_max; /**< Max number of segments per whole packet. */
+ uint16_t nb_mtu_seg_max; /**< Max number of segments per one MTU */
};
/**
@@ -1188,6 +1191,11 @@ typedef uint16_t (*eth_tx_burst_t)(void *txq,
uint16_t nb_pkts);
/**< @internal Send output packets on a transmit queue of an Ethernet device. */
+typedef uint16_t (*eth_tx_prep_t)(void *txq,
+ struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts);
+/**< @internal Prepare output packets on a transmit queue of an Ethernet device. */
+
typedef int (*flow_ctrl_get_t)(struct rte_eth_dev *dev,
struct rte_eth_fc_conf *fc_conf);
/**< @internal Get current flow control parameter on an Ethernet device */
@@ -1622,6 +1630,7 @@ struct rte_eth_rxtx_callback {
struct rte_eth_dev {
eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
+ eth_tx_prep_t tx_pkt_prep; /**< Pointer to PMD transmit prepare function. */
struct rte_eth_dev_data *data; /**< Pointer to device data */
const struct eth_driver *driver;/**< Driver for this device */
const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
@@ -2816,6 +2825,94 @@ rte_eth_tx_burst(uint8_t port_id, uint16_t queue_id,
return (*dev->tx_pkt_burst)(dev->data->tx_queues[queue_id], tx_pkts, nb_pkts);
}
+/**
+ * Process a burst of output packets on a transmit queue of an Ethernet device.
+ *
+ * The rte_eth_tx_prep() function is invoked to prepare output packets to be
+ * transmitted on the output queue *queue_id* of the Ethernet device designated
+ * by its *port_id*.
+ * The *nb_pkts* parameter is the number of packets to be prepared which are
+ * supplied in the *tx_pkts* array of *rte_mbuf* structures, each of them
+ * allocated from a pool created with rte_pktmbuf_pool_create().
+ * For each packet to send, the rte_eth_tx_prep() function performs
+ * the following operations:
+ *
+ * - Check if packet meets devices requirements for tx offloads.
+ *
+ * - Check limitations about number of segments.
+ *
+ * - Check additional requirements when debug is enabled.
+ *
+ * - Update and/or reset required checksums when tx offload is set for packet.
+ *
+ * The rte_eth_tx_prep() function returns the number of packets ready to be
+ * sent. A return value equal to *nb_pkts* means that all packets are valid and
+ * ready to be sent.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * The value must be a valid port id.
+ * @param queue_id
+ * The index of the transmit queue through which output packets must be
+ * sent.
+ * The value must be in the range [0, nb_tx_queue - 1] previously supplied
+ * to rte_eth_dev_configure().
+ * @param tx_pkts
+ * The address of an array of *nb_pkts* pointers to *rte_mbuf* structures
+ * which contain the output packets.
+ * @param nb_pkts
+ * The maximum number of packets to process.
+ * @return
+ * The number of packets correct and ready to be sent. The return value can be
+ * less than the value of the *tx_pkts* parameter when some packet doesn't
+ * meet devices requirements with rte_errno set appropriately.
+ */
+
+#ifdef RTE_ETHDEV_TX_PREP
+
+static inline uint16_t
+rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id, struct rte_mbuf **tx_pkts,
+ uint16_t nb_pkts)
+{
+ struct rte_eth_dev *dev;
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ if (!rte_eth_dev_is_valid_port(port_id)) {
+ RTE_PMD_DEBUG_TRACE("Invalid TX port_id=%d\n", port_id);
+ rte_errno = -EINVAL;
+ return 0;
+ }
+#endif
+
+ dev = &rte_eth_devices[port_id];
+
+ if (!dev->tx_pkt_prep)
+ return nb_pkts;
+
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+ if (queue_id >= dev->data->nb_tx_queues) {
+ RTE_PMD_DEBUG_TRACE("Invalid TX queue_id=%d\n", queue_id);
+ rte_errno = -EINVAL;
+ return 0;
+ }
+#endif
+
+ return (*dev->tx_pkt_prep)(dev->data->tx_queues[queue_id],
+ tx_pkts, nb_pkts);
+}
+
+#else
+
+static inline uint16_t
+rte_eth_tx_prep(__rte_unused uint8_t port_id, __rte_unused uint16_t queue_id,
+ __rte_unused struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
+{
+ return nb_pkts;
+}
+
+#endif
+
typedef void (*buffer_tx_error_fn)(struct rte_mbuf **unsent, uint16_t count,
void *userdata);
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 109e666..ff9e749 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -283,6 +283,19 @@ extern "C" {
*/
#define PKT_TX_OUTER_IPV6 (1ULL << 60)
+/**
+ * Bit Mask of all supported packet Tx offload features flags, which can be set
+ * for packet.
+ */
+#define PKT_TX_OFFLOAD_MASK ( \
+ PKT_TX_IP_CKSUM | \
+ PKT_TX_L4_MASK | \
+ PKT_TX_OUTER_IP_CKSUM | \
+ PKT_TX_TCP_SEG | \
+ PKT_TX_QINQ_PKT | \
+ PKT_TX_VLAN_PKT | \
+ PKT_TX_TUNNEL_MASK)
+
#define __RESERVED (1ULL << 61) /**< reserved for future mbuf use */
#define IND_ATTACHED_MBUF (1ULL << 62) /**< Indirect attached mbuf */
@@ -1647,6 +1660,57 @@ static inline int rte_pktmbuf_chain(struct rte_mbuf *head, struct rte_mbuf *tail
}
/**
+ * Validate general requirements for tx offload in mbuf.
+ *
+ * This function checks correctness and completeness of Tx offload settings.
+ *
+ * @param m
+ * The packet mbuf to be validated.
+ * @return
+ * 0 if packet is valid
+ */
+static inline int
+rte_validate_tx_offload(const struct rte_mbuf *m)
+{
+ uint64_t ol_flags = m->ol_flags;
+ uint64_t inner_l3_offset = m->l2_len;
+
+ /* Does packet set any of available offloads? */
+ if (!(ol_flags & PKT_TX_OFFLOAD_MASK))
+ return 0;
+
+ if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
+ inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
+
+ /* Headers are fragmented */
+ if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
+ return -ENOTSUP;
+
+ /* IP checksum can be counted only for IPv4 packet */
+ if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6))
+ return -EINVAL;
+
+ /* IP type not set when required */
+ if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))
+ if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
+ return -EINVAL;
+
+ /* Check requirements for TSO packet */
+ if (ol_flags & PKT_TX_TCP_SEG)
+ if ((m->tso_segsz == 0) ||
+ ((ol_flags & PKT_TX_IPV4) &&
+ !(ol_flags & PKT_TX_IP_CKSUM)))
+ return -EINVAL;
+
+ /* PKT_TX_OUTER_IP_CKSUM set for non outer IPv4 packet. */
+ if ((ol_flags & PKT_TX_OUTER_IP_CKSUM) &&
+ !(ol_flags & PKT_TX_OUTER_IPV4))
+ return -EINVAL;
+
+ return 0;
+}
+
+/**
* Dump an mbuf structure to a file.
*
* Dump all fields for the given packet mbuf and all its associated
diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
index d4156ae..90af335 100644
--- a/lib/librte_net/rte_net.h
+++ b/lib/librte_net/rte_net.h
@@ -38,6 +38,11 @@
extern "C" {
#endif
+#include <rte_ip.h>
+#include <rte_udp.h>
+#include <rte_tcp.h>
+#include <rte_sctp.h>
+
/**
* Structure containing header lengths associated to a packet, filled
* by rte_net_get_ptype().
@@ -86,6 +91,86 @@ struct rte_net_hdr_lens {
uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
struct rte_net_hdr_lens *hdr_lens, uint32_t layers);
+/**
+ * Fix pseudo header checksum
+ *
+ * This function fixes pseudo header checksum for TSO and non-TSO tcp/udp in
+ * provided mbufs packet data.
+ *
+ * - for non-TSO tcp/udp packets full pseudo-header checksum is counted and set
+ * in packet data,
+ * - for TSO the IP payload length is not included in pseudo header.
+ *
+ * This function expects that used headers are in the first data segment of
+ * mbuf, and are not fragmented.
+ *
+ * @param m
+ * The packet mbuf to be validated.
+ * @return
+ * 0 if checksum is initialized properly
+ */
+static inline int
+rte_phdr_cksum_fix(struct rte_mbuf *m)
+{
+ struct ipv4_hdr *ipv4_hdr;
+ struct ipv6_hdr *ipv6_hdr;
+ struct tcp_hdr *tcp_hdr;
+ struct udp_hdr *udp_hdr;
+ uint64_t ol_flags = m->ol_flags;
+ uint64_t inner_l3_offset = m->l2_len;
+
+ if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
+ inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
+
+ if ((ol_flags & PKT_TX_UDP_CKSUM) == PKT_TX_UDP_CKSUM) {
+ if (ol_flags & PKT_TX_IPV4) {
+ ipv4_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
+ inner_l3_offset);
+
+ if (ol_flags & PKT_TX_IP_CKSUM)
+ ipv4_hdr->hdr_checksum = 0;
+
+ udp_hdr = (struct udp_hdr *)((char *)ipv4_hdr +
+ m->l3_len);
+ udp_hdr->dgram_cksum = rte_ipv4_phdr_cksum(ipv4_hdr,
+ ol_flags);
+ } else {
+ ipv6_hdr = rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *,
+ inner_l3_offset);
+ /* non-TSO udp */
+ udp_hdr = rte_pktmbuf_mtod_offset(m, struct udp_hdr *,
+ inner_l3_offset + m->l3_len);
+ udp_hdr->dgram_cksum = rte_ipv6_phdr_cksum(ipv6_hdr,
+ ol_flags);
+ }
+ } else if ((ol_flags & PKT_TX_TCP_CKSUM) ||
+ (ol_flags & PKT_TX_TCP_SEG)) {
+ if (ol_flags & PKT_TX_IPV4) {
+ ipv4_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
+ inner_l3_offset);
+
+ if (ol_flags & PKT_TX_IP_CKSUM)
+ ipv4_hdr->hdr_checksum = 0;
+
+ /* non-TSO tcp or TSO */
+ tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr +
+ m->l3_len);
+ tcp_hdr->cksum = rte_ipv4_phdr_cksum(ipv4_hdr,
+ ol_flags);
+ } else {
+ ipv6_hdr = rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *,
+ inner_l3_offset);
+ /* non-TSO tcp or TSO */
+ tcp_hdr = rte_pktmbuf_mtod_offset(m, struct tcp_hdr *,
+ inner_l3_offset + m->l3_len);
+ tcp_hdr->cksum = rte_ipv6_phdr_cksum(ipv6_hdr,
+ ol_flags);
+ }
+ }
+
+ return 0;
+}
+
#ifdef __cplusplus
}
#endif
--
1.7.9.5
^ permalink raw reply related
* [PATCH v9 0/6] add Tx preparation
From: Tomasz Kulasek @ 2016-10-24 14:05 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, olivier.matz
In-Reply-To: <1477061177-15828-1-git-send-email-tomaszx.kulasek@intel.com>
>From 35b09a978d244092337b6f46fd1309f8c733bb6b Mon Sep 17 00:00:00 2001
From: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Date: Fri, 14 Oct 2016 16:10:35 +0200
Subject: [PATCH v6 0/6] add Tx preparation
As discussed in that thread:
http://dpdk.org/ml/archives/dev/2015-September/023603.html
Different NIC models depending on HW offload requested might impose
different requirements on packets to be TX-ed in terms of:
- Max number of fragments per packet allowed
- Max number of fragments per TSO segments
- The way pseudo-header checksum should be pre-calculated
- L3/L4 header fields filling
- etc.
MOTIVATION:
-----------
1) Some work cannot (and didn't should) be done in rte_eth_tx_burst.
However, this work is sometimes required, and now, it's an
application issue.
2) Different hardware may have different requirements for TX offloads,
other subset can be supported and so on.
3) Some parameters (e.g. number of segments in ixgbe driver) may hung
device. These parameters may be vary for different devices.
For example i40e HW allows 8 fragments per packet, but that is after
TSO segmentation. While ixgbe has a 38-fragment pre-TSO limit.
4) Fields in packet may require different initialization (like e.g. will
require pseudo-header checksum precalculation, sometimes in a
different way depending on packet type, and so on). Now application
needs to care about it.
5) Using additional API (rte_eth_tx_prep) before rte_eth_tx_burst let to
prepare packet burst in acceptable form for specific device.
6) Some additional checks may be done in debug mode keeping tx_burst
implementation clean.
PROPOSAL:
---------
To help user to deal with all these varieties we propose to:
1) Introduce rte_eth_tx_prep() function to do necessary preparations of
packet burst to be safely transmitted on device for desired HW
offloads (set/reset checksum field according to the hardware
requirements) and check HW constraints (number of segments per
packet, etc).
While the limitations and requirements may differ for devices, it
requires to extend rte_eth_dev structure with new function pointer
"tx_pkt_prep" which can be implemented in the driver to prepare and
verify packets, in devices specific way, before burst, what should to
prevent application to send malformed packets.
2) Also new fields will be introduced in rte_eth_desc_lim:
nb_seg_max and nb_mtu_seg_max, providing an information about max
segments in TSO and non-TSO packets acceptable by device.
This information is useful for application to not create/limit
malicious packet.
APPLICATION (CASE OF USE):
--------------------------
1) Application should to initialize burst of packets to send, set
required tx offload flags and required fields, like l2_len, l3_len,
l4_len, and tso_segsz
2) Application passes burst to the rte_eth_tx_prep to check conditions
required to send packets through the NIC.
3) The result of rte_eth_tx_prep can be used to send valid packets
and/or restore invalid if function fails.
e.g.
for (i = 0; i < nb_pkts; i++) {
/* initialize or process packet */
bufs[i]->tso_segsz = 800;
bufs[i]->ol_flags = PKT_TX_TCP_SEG | PKT_TX_IPV4
| PKT_TX_IP_CKSUM;
bufs[i]->l2_len = sizeof(struct ether_hdr);
bufs[i]->l3_len = sizeof(struct ipv4_hdr);
bufs[i]->l4_len = sizeof(struct tcp_hdr);
}
/* Prepare burst of TX packets */
nb_prep = rte_eth_tx_prep(port, 0, bufs, nb_pkts);
if (nb_prep < nb_pkts) {
printf("tx_prep failed\n");
/* nb_prep indicates here first invalid packet. rte_eth_tx_prep
* can be used on remaining packets to find another ones.
*/
}
/* Send burst of TX packets */
nb_tx = rte_eth_tx_burst(port, 0, bufs, nb_prep);
/* Free any unsent packets. */
v9 changes:
- fixed headers structure fragmentation check
- moved fragmentation check into rte_validate_tx_offload()
v8 changes:
- mbuf argument in rte_validate_tx_offload declared as const
v7 changes:
- comments reworded/added
- changed errno values returned from Tx prep API
- added check in rte_phdr_cksum_fix if headers are in the first
data segment and can be safetly modified
- moved rte_validate_tx_offload to rte_mbuf
- moved rte_phdr_cksum_fix to rte_net.h
- removed rte_pkt.h new file as useless
v6 changes:
- added performance impact test results to the patch description
v5 changes:
- rebased csum engine modification
- added information to the csum engine about performance tests
- some performance improvements
v4 changes:
- tx_prep is now set to default behavior (NULL) for simple/vector path
in fm10k, i40e and ixgbe drivers to increase performance, when
Tx offloads are not intentionally available
v3 changes:
- reworked csum testpmd engine instead adding new one,
- fixed checksum initialization procedure to include also outer
checksum offloads,
- some minor formattings and optimalizations
v2 changes:
- rte_eth_tx_prep() returns number of packets when device doesn't
support tx_prep functionality,
- introduced CONFIG_RTE_ETHDEV_TX_PREP allowing to turn off tx_prep
Tomasz Kulasek (6):
ethdev: add Tx preparation
e1000: add Tx preparation
fm10k: add Tx preparation
i40e: add Tx preparation
ixgbe: add Tx preparation
testpmd: use Tx preparation in csum engine
app/test-pmd/csumonly.c | 36 +++++---------
config/common_base | 1 +
drivers/net/e1000/e1000_ethdev.h | 11 +++++
drivers/net/e1000/em_ethdev.c | 5 +-
drivers/net/e1000/em_rxtx.c | 48 ++++++++++++++++++-
drivers/net/e1000/igb_ethdev.c | 4 ++
drivers/net/e1000/igb_rxtx.c | 52 +++++++++++++++++++-
drivers/net/fm10k/fm10k.h | 6 +++
drivers/net/fm10k/fm10k_ethdev.c | 5 ++
drivers/net/fm10k/fm10k_rxtx.c | 50 +++++++++++++++++++-
drivers/net/i40e/i40e_ethdev.c | 3 ++
drivers/net/i40e/i40e_rxtx.c | 72 +++++++++++++++++++++++++++-
drivers/net/i40e/i40e_rxtx.h | 8 ++++
drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++
drivers/net/ixgbe/ixgbe_ethdev.h | 5 +-
drivers/net/ixgbe/ixgbe_rxtx.c | 58 ++++++++++++++++++++++-
drivers/net/ixgbe/ixgbe_rxtx.h | 2 +
lib/librte_ether/rte_ethdev.h | 97 ++++++++++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf.h | 64 +++++++++++++++++++++++++
lib/librte_net/rte_net.h | 85 +++++++++++++++++++++++++++++++++
20 files changed, 585 insertions(+), 30 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH v4 00/32] net/qede: update qede pmd to 1.2.0.1 and enable by default
From: Bruce Richardson @ 2016-10-24 13:41 UTC (permalink / raw)
To: Rasesh Mody; +Cc: ferruh.yigit, thomas.monjalon, dev, Dept-EngDPDKDev
In-Reply-To: <1476850306-2141-1-git-send-email-rasesh.mody@qlogic.com>
On Tue, Oct 18, 2016 at 09:11:14PM -0700, Rasesh Mody wrote:
> Hi,
>
> This patch set includes changes to update the base driver, work with
> newer FW 8.10.9.0, adds new features, includes enhancements and code
> cleanup, provides bug fixes and updates documentation for the QEDE
> poll mode driver.
>
> It enables QEDE PMD in the dpdk config by default. The dependency on
> external library libz has been addressed.
>
> The patch set updates the QEDE PMD to 1.2.0.1.
>
> Review comments received for v3 have been addressed.
>
> Please apply to DPDK tree for v16.11 release.
>
> Thanks!
> Rasesh
>
> Harish Patil (14):
> net/qede: change signature of MCP command API
> net/qede: serialize access to MFW mbox
> net/qede: add NIC selftest and query sensor info support
> net/qede: fix port (re)configuration issue
> net/qede/base: allow MTU change via vport-update
> net/qede: add missing 100G link speed capability
> net/qede: remove unused/dead code
> net/qede: fixes for VLAN filters
> net/qede: add enable/disable VLAN filtering
> net/qede: fix RSS related issues
> net/qede/base: add support to initiate PF FLR
> net/qede: skip slowpath polling for 100G VF device
> net/qede: fix driver version string
> net/qede: fix status block index for VF queues
>
> Rasesh Mody (16):
> net/qede/base: add new init files and rearrange the code
> net/qede/base: formatting changes
> net/qede: use FW CONFIG defines as needed
> net/qede/base: add HSI changes and register defines
> net/qede/base: add attention formatting string
> net/qede/base: additional formatting/comment changes
> net/qede: fix 32 bit compilation
> net/qede/base: update base driver
> net/qede/base: rename structure and defines
> net/qede/base: comment enhancements
> net/qede/base: add MFW crash dump support
> net/qede/base: change Rx Tx queue start APIs
> net/qede: add support for queue statistics
> net/qede: remove zlib dependency and enable PMD by default
> doc: update qede pmd documentation
> net/qede: update driver version
>
> Sony Chacko (2):
> net/qede: enable support for unequal number of Rx/Tx queues
> net/qede: add scatter gather support
>
Patchset applied to dpdk_next_net/rel_16_11
Thanks,
/Bruce
^ permalink raw reply
* Re: [PATCH 2/2] app/test: use correct offsets in AES perf test
From: Jain, Deepak K @ 2016-10-24 13:35 UTC (permalink / raw)
To: Trahe, Fiona, dev@dpdk.org
Cc: De Lara Guarch, Pablo, Griffin, John, Kusztal, ArkadiuszX
In-Reply-To: <1477310400-4934-3-git-send-email-fiona.trahe@intel.com>
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Monday, October 24, 2016 1:00 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Griffin, John <john.griffin@intel.com>; Jain,
> Deepak K <deepak.k.jain@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH 2/2] app/test: use correct offsets in AES perf test
>
> offsets for digest and data need to be adjusted to take prepended IV into
> account
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
> app/test/test_cryptodev_perf.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/app/test/test_cryptodev_perf.c
> b/app/test/test_cryptodev_perf.c index e05e11b..53dd8f5 100644
> --- a/app/test/test_cryptodev_perf.c
> +++ b/app/test/test_cryptodev_perf.c
> @@ -2714,26 +2714,27 @@ test_perf_set_crypto_op_aes(struct
> rte_crypto_op *op, struct rte_mbuf *m,
>
> --
> 2.5.0
Acked-by: Deepak Kumar Jain <deepak.k.jain@intel.com>
^ permalink raw reply
* Re: [PATCH 1/2] crypto/qat: rework request builder for performance
From: Jain, Deepak K @ 2016-10-24 13:34 UTC (permalink / raw)
To: Trahe, Fiona, dev@dpdk.org
Cc: De Lara Guarch, Pablo, Griffin, John, Kusztal, ArkadiuszX
In-Reply-To: <1477310400-4934-2-git-send-email-fiona.trahe@intel.com>
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Monday, October 24, 2016 1:00 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Griffin, John <john.griffin@intel.com>; Jain,
> Deepak K <deepak.k.jain@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH 1/2] crypto/qat: rework request builder for performance
>
> QAT PMD adjusts the buffer start address and offsets passed to the device
> so that the DMAs in and out of the device are 64-byte aligned.
> This gives more consistent throughput, which had been variable depending
> on how the application set up the mbuf.
> The message builder code had to be considerably re-factored to do this
> efficiently.
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
> drivers/crypto/qat/qat_adf/icp_qat_hw.h | 5 +
> drivers/crypto/qat/qat_adf/qat_algs.h | 1 +
> drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 2 +
> drivers/crypto/qat/qat_crypto.c | 239 ++++++++++++++++-------
> 4 files changed, 176 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_adf/icp_qat_hw.h
> b/drivers/crypto/qat/qat_adf/icp_qat_hw.h
> index a08094f..ebe245f 100644
> --- a/drivers/crypto/qat/qat_adf/icp_qat_hw.h
> --
> 2.5.0
Acked-By: Deepak Kumar Jain <deepak.k.jain@intel.com>
^ permalink raw reply
* Re: [PATCH v2] net/mlx5: fix init on secondary process
From: Bruce Richardson @ 2016-10-24 13:26 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Ferruh Yigit, Olivier Gournet, dev, Nelio Laranjeiro
In-Reply-To: <20161017141859.GB5733@6wind.com>
On Mon, Oct 17, 2016 at 04:18:59PM +0200, Adrien Mazarguil wrote:
> On Mon, Oct 17, 2016 at 02:52:39PM +0100, Ferruh Yigit wrote:
> > Hi Adrien,
> >
> > On 10/17/2016 1:56 PM, Olivier Gournet wrote:
> > > Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
> > > Fixes: 21c8bb4928c9 ("net/mlx5: split Tx queue structure")
> > >
> > > Signed-off-by: Olivier Gournet <ogournet@corp.free.fr>
> >
> > According your comment on previous version of it, I think you have your
> > Ack on this patch, but can you please confirm?
>
> Yes it's fine, thanks.
>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
Applied to dpdk-next-net/rel_16_11
/Bruce
^ permalink raw reply
* Re: [PATCH v8 1/6] ethdev: add Tx preparation
From: Ananyev, Konstantin @ 2016-10-24 12:56 UTC (permalink / raw)
To: Kulasek, TomaszX, dev@dpdk.org; +Cc: olivier.matz@6wind.com
In-Reply-To: <3042915272161B4EB253DA4D77EB373A14F44670@IRSMSX102.ger.corp.intel.com>
> -----Original Message-----
> From: Kulasek, TomaszX
> Sent: Monday, October 24, 2016 1:49 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com
> Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
>
> Hi Konstantin,
>
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Monday, October 24, 2016 14:15
> > To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> > Cc: olivier.matz@6wind.com
> > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
> >
> > Hi Tomasz,
> >
>
> [...]
>
> > >
> > > +/**
> > > + * Fix pseudo header checksum
> > > + *
> > > + * This function fixes pseudo header checksum for TSO and non-TSO
> > > +tcp/udp in
> > > + * provided mbufs packet data.
> > > + *
> > > + * - for non-TSO tcp/udp packets full pseudo-header checksum is counted
> > and set
> > > + * in packet data,
> > > + * - for TSO the IP payload length is not included in pseudo header.
> > > + *
> > > + * This function expects that used headers are in the first data
> > > +segment of
> > > + * mbuf, and are not fragmented.
> > > + *
> > > + * @param m
> > > + * The packet mbuf to be validated.
> > > + * @return
> > > + * 0 if checksum is initialized properly
> > > + */
> > > +static inline int
> > > +rte_phdr_cksum_fix(struct rte_mbuf *m) {
> > > + struct ipv4_hdr *ipv4_hdr;
> > > + struct ipv6_hdr *ipv6_hdr;
> > > + struct tcp_hdr *tcp_hdr;
> > > + struct udp_hdr *udp_hdr;
> > > + uint64_t ol_flags = m->ol_flags;
> > > + uint64_t inner_l3_offset = m->l2_len;
> > > +
> > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> > > + inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> > > +
> > > + /* headers are fragmented */
> > > + if (unlikely(rte_pktmbuf_data_len(m) >= inner_l3_offset + m->l3_len
> > +
> > > + m->l4_len))
> >
> > Might be better to move that check into rte_validate_tx_offload(), so it
> > would be called only when TX_DEBUG is on.
>
> While unfragmented headers are not general requirements for Tx offloads, and this requirement is for this particular implementation,
> maybe for performance reasons will be better to keep it here, and just add #if DEBUG to leave rte_validate_tx_offload more generic.
Hmm and what is the advantage to pollute that code with more ifdefs?
Again, why unfragmented headers are not general requirements?
As long as DPDK pseudo-headear csum calculation routines can't handle fragmented case,
it pretty much is a general requirement, no?
Konstantin
>
> > Another thing, shouldn't it be:
> > if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len) ?
>
> Yes, it should.
>
> > Konstantin
> >
>
> Tomasz
^ permalink raw reply
* Re: [PATCH v8 1/6] ethdev: add Tx preparation
From: Kulasek, TomaszX @ 2016-10-24 12:49 UTC (permalink / raw)
To: Ananyev, Konstantin, dev@dpdk.org; +Cc: olivier.matz@6wind.com
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CB086@irsmsx105.ger.corp.intel.com>
Hi Konstantin,
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Monday, October 24, 2016 14:15
> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com
> Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
>
> Hi Tomasz,
>
[...]
> >
> > +/**
> > + * Fix pseudo header checksum
> > + *
> > + * This function fixes pseudo header checksum for TSO and non-TSO
> > +tcp/udp in
> > + * provided mbufs packet data.
> > + *
> > + * - for non-TSO tcp/udp packets full pseudo-header checksum is counted
> and set
> > + * in packet data,
> > + * - for TSO the IP payload length is not included in pseudo header.
> > + *
> > + * This function expects that used headers are in the first data
> > +segment of
> > + * mbuf, and are not fragmented.
> > + *
> > + * @param m
> > + * The packet mbuf to be validated.
> > + * @return
> > + * 0 if checksum is initialized properly
> > + */
> > +static inline int
> > +rte_phdr_cksum_fix(struct rte_mbuf *m) {
> > + struct ipv4_hdr *ipv4_hdr;
> > + struct ipv6_hdr *ipv6_hdr;
> > + struct tcp_hdr *tcp_hdr;
> > + struct udp_hdr *udp_hdr;
> > + uint64_t ol_flags = m->ol_flags;
> > + uint64_t inner_l3_offset = m->l2_len;
> > +
> > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> > + inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> > +
> > + /* headers are fragmented */
> > + if (unlikely(rte_pktmbuf_data_len(m) >= inner_l3_offset + m->l3_len
> +
> > + m->l4_len))
>
> Might be better to move that check into rte_validate_tx_offload(), so it
> would be called only when TX_DEBUG is on.
While unfragmented headers are not general requirements for Tx offloads, and this requirement is for this particular implementation, maybe for performance reasons will be better to keep it here, and just add #if DEBUG to leave rte_validate_tx_offload more generic.
> Another thing, shouldn't it be:
> if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len) ?
Yes, it should.
> Konstantin
>
Tomasz
^ permalink raw reply
* Re: [PATCH v8 1/6] ethdev: add Tx preparation
From: Ananyev, Konstantin @ 2016-10-24 12:14 UTC (permalink / raw)
To: Kulasek, TomaszX, dev@dpdk.org; +Cc: olivier.matz@6wind.com
In-Reply-To: <1477061177-15828-2-git-send-email-tomaszx.kulasek@intel.com>
Hi Tomasz,
>
> /**
> + * Validate general requirements for tx offload in mbuf.
> + *
> + * This function checks correctness and completeness of Tx offload settings.
> + *
> + * @param m
> + * The packet mbuf to be validated.
> + * @return
> + * 0 if packet is valid
> + */
> +static inline int
> +rte_validate_tx_offload(const struct rte_mbuf *m)
> +{
> + uint64_t ol_flags = m->ol_flags;
> +
> + /* Does packet set any of available offloads? */
> + if (!(ol_flags & PKT_TX_OFFLOAD_MASK))
> + return 0;
> +
> + /* IP checksum can be counted only for IPv4 packet */
> + if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6))
> + return -EINVAL;
> +
> + /* IP type not set when required */
> + if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))
> + if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
> + return -EINVAL;
> +
> + /* Check requirements for TSO packet */
> + if (ol_flags & PKT_TX_TCP_SEG)
> + if ((m->tso_segsz == 0) ||
> + ((ol_flags & PKT_TX_IPV4) &&
> + !(ol_flags & PKT_TX_IP_CKSUM)))
> + return -EINVAL;
> +
> + /* PKT_TX_OUTER_IP_CKSUM set for non outer IPv4 packet. */
> + if ((ol_flags & PKT_TX_OUTER_IP_CKSUM) &&
> + !(ol_flags & PKT_TX_OUTER_IPV4))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/**
> * Dump an mbuf structure to a file.
> *
> * Dump all fields for the given packet mbuf and all its associated
> diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
> index d4156ae..79669d7 100644
> --- a/lib/librte_net/rte_net.h
> +++ b/lib/librte_net/rte_net.h
> @@ -38,6 +38,11 @@
> extern "C" {
> #endif
>
> +#include <rte_ip.h>
> +#include <rte_udp.h>
> +#include <rte_tcp.h>
> +#include <rte_sctp.h>
> +
> /**
> * Structure containing header lengths associated to a packet, filled
> * by rte_net_get_ptype().
> @@ -86,6 +91,91 @@ struct rte_net_hdr_lens {
> uint32_t rte_net_get_ptype(const struct rte_mbuf *m,
> struct rte_net_hdr_lens *hdr_lens, uint32_t layers);
>
> +/**
> + * Fix pseudo header checksum
> + *
> + * This function fixes pseudo header checksum for TSO and non-TSO tcp/udp in
> + * provided mbufs packet data.
> + *
> + * - for non-TSO tcp/udp packets full pseudo-header checksum is counted and set
> + * in packet data,
> + * - for TSO the IP payload length is not included in pseudo header.
> + *
> + * This function expects that used headers are in the first data segment of
> + * mbuf, and are not fragmented.
> + *
> + * @param m
> + * The packet mbuf to be validated.
> + * @return
> + * 0 if checksum is initialized properly
> + */
> +static inline int
> +rte_phdr_cksum_fix(struct rte_mbuf *m)
> +{
> + struct ipv4_hdr *ipv4_hdr;
> + struct ipv6_hdr *ipv6_hdr;
> + struct tcp_hdr *tcp_hdr;
> + struct udp_hdr *udp_hdr;
> + uint64_t ol_flags = m->ol_flags;
> + uint64_t inner_l3_offset = m->l2_len;
> +
> + if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> + inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> +
> + /* headers are fragmented */
> + if (unlikely(rte_pktmbuf_data_len(m) >= inner_l3_offset + m->l3_len +
> + m->l4_len))
Might be better to move that check into rte_validate_tx_offload(),
so it would be called only when TX_DEBUG is on.
Another thing, shouldn't it be:
if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len)
?
Konstantin
> + return -ENOTSUP;
> +
> + if ((ol_flags & PKT_TX_UDP_CKSUM) == PKT_TX_UDP_CKSUM) {
> + if (ol_flags & PKT_TX_IPV4) {
> + ipv4_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
> + inner_l3_offset);
> +
> + if (ol_flags & PKT_TX_IP_CKSUM)
> + ipv4_hdr->hdr_checksum = 0;
> +
> + udp_hdr = (struct udp_hdr *)((char *)ipv4_hdr +
> + m->l3_len);
> + udp_hdr->dgram_cksum = rte_ipv4_phdr_cksum(ipv4_hdr,
> + ol_flags);
> + } else {
> + ipv6_hdr = rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *,
> + inner_l3_offset);
> + /* non-TSO udp */
> + udp_hdr = rte_pktmbuf_mtod_offset(m, struct udp_hdr *,
> + inner_l3_offset + m->l3_len);
> + udp_hdr->dgram_cksum = rte_ipv6_phdr_cksum(ipv6_hdr,
> + ol_flags);
> + }
> + } else if ((ol_flags & PKT_TX_TCP_CKSUM) ||
> + (ol_flags & PKT_TX_TCP_SEG)) {
> + if (ol_flags & PKT_TX_IPV4) {
> + ipv4_hdr = rte_pktmbuf_mtod_offset(m, struct ipv4_hdr *,
> + inner_l3_offset);
> +
> + if (ol_flags & PKT_TX_IP_CKSUM)
> + ipv4_hdr->hdr_checksum = 0;
> +
> + /* non-TSO tcp or TSO */
> + tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr +
> + m->l3_len);
> + tcp_hdr->cksum = rte_ipv4_phdr_cksum(ipv4_hdr,
> + ol_flags);
> + } else {
> + ipv6_hdr = rte_pktmbuf_mtod_offset(m, struct ipv6_hdr *,
> + inner_l3_offset);
> + /* non-TSO tcp or TSO */
> + tcp_hdr = rte_pktmbuf_mtod_offset(m, struct tcp_hdr *,
> + inner_l3_offset + m->l3_len);
> + tcp_hdr->cksum = rte_ipv6_phdr_cksum(ipv6_hdr,
> + ol_flags);
> + }
> + }
> +
> + return 0;
> +}
> +
> #ifdef __cplusplus
> }
> #endif
> --
> 1.7.9.5
^ permalink raw reply
* [PATCH 2/2] app/test: use correct offsets in AES perf test
From: Fiona Trahe @ 2016-10-24 12:00 UTC (permalink / raw)
To: dev
Cc: pablo.de.lara.guarch, fiona.trahe, john.griffin, deepak.k.jain,
arkadiuszx.kusztal
In-Reply-To: <1477310400-4934-1-git-send-email-fiona.trahe@intel.com>
offsets for digest and data need to be adjusted
to take prepended IV into account
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
app/test/test_cryptodev_perf.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
index e05e11b..53dd8f5 100644
--- a/app/test/test_cryptodev_perf.c
+++ b/app/test/test_cryptodev_perf.c
@@ -2714,26 +2714,27 @@ test_perf_set_crypto_op_aes(struct rte_crypto_op *op, struct rte_mbuf *m,
}
/* Authentication Parameters */
- op->sym->auth.digest.data = (uint8_t *)m->buf_addr +
- (m->data_off + data_len);
- op->sym->auth.digest.phys_addr = rte_pktmbuf_mtophys_offset(m, data_len);
+ op->sym->auth.digest.data = rte_pktmbuf_mtod_offset(m, uint8_t *,
+ AES_CIPHER_IV_LENGTH + data_len);
+ op->sym->auth.digest.phys_addr = rte_pktmbuf_mtophys_offset(m,
+ AES_CIPHER_IV_LENGTH + data_len);
op->sym->auth.digest.length = digest_len;
op->sym->auth.aad.data = aes_iv;
op->sym->auth.aad.length = AES_CIPHER_IV_LENGTH;
/* Cipher Parameters */
- op->sym->cipher.iv.data = (uint8_t *)m->buf_addr + m->data_off;
+ op->sym->cipher.iv.data = rte_pktmbuf_mtod(m, uint8_t *);
op->sym->cipher.iv.phys_addr = rte_pktmbuf_mtophys(m);
op->sym->cipher.iv.length = AES_CIPHER_IV_LENGTH;
rte_memcpy(op->sym->cipher.iv.data, aes_iv, AES_CIPHER_IV_LENGTH);
/* Data lengths/offsets Parameters */
- op->sym->auth.data.offset = 0;
+ op->sym->auth.data.offset = AES_CIPHER_IV_LENGTH;
op->sym->auth.data.length = data_len;
- op->sym->cipher.data.offset = AES_BLOCK_SIZE;
- op->sym->cipher.data.length = data_len - AES_BLOCK_SIZE;
+ op->sym->cipher.data.offset = AES_CIPHER_IV_LENGTH;
+ op->sym->cipher.data.length = data_len;
op->sym->m_src = m;
--
2.5.0
^ permalink raw reply related
* [PATCH 0/2] crypto/qat: performance optimisation
From: Fiona Trahe @ 2016-10-24 11:59 UTC (permalink / raw)
To: dev
Cc: pablo.de.lara.guarch, fiona.trahe, john.griffin, deepak.k.jain,
arkadiuszx.kusztal
QAT PMD adjusts the buffer start address and offsets passed
to the device so that the DMAs in and out of the device are
64-byte aligned.
This gives more consistent throughput, which had been
variable depending on how the application set up the mbuf.
The message builder code had to be considerably re-factored
to do this efficiently.
Also performance test not taking IV prepend offsets
into account were corrected.
Fiona Trahe (2):
crypto/qat: rework request builder for performance
app/test: use correct offsets in AES perf test
app/test/test_cryptodev_perf.c | 15 +-
drivers/crypto/qat/qat_adf/icp_qat_hw.h | 5 +
drivers/crypto/qat/qat_adf/qat_algs.h | 1 +
drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 2 +
drivers/crypto/qat/qat_crypto.c | 242 ++++++++++++++++-------
5 files changed, 185 insertions(+), 80 deletions(-)
--
2.5.0
^ permalink raw reply
* [PATCH 1/2] crypto/qat: rework request builder for performance
From: Fiona Trahe @ 2016-10-24 11:59 UTC (permalink / raw)
To: dev
Cc: pablo.de.lara.guarch, fiona.trahe, john.griffin, deepak.k.jain,
arkadiuszx.kusztal
In-Reply-To: <1477310400-4934-1-git-send-email-fiona.trahe@intel.com>
QAT PMD adjusts the buffer start address and offsets passed
to the device so that the DMAs in and out of the device are
64-byte aligned.
This gives more consistent throughput, which had been
variable depending on how the application set up the mbuf.
The message builder code had to be considerably re-factored
to do this efficiently.
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
drivers/crypto/qat/qat_adf/icp_qat_hw.h | 5 +
drivers/crypto/qat/qat_adf/qat_algs.h | 1 +
drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 2 +
drivers/crypto/qat/qat_crypto.c | 239 ++++++++++++++++-------
4 files changed, 176 insertions(+), 71 deletions(-)
diff --git a/drivers/crypto/qat/qat_adf/icp_qat_hw.h b/drivers/crypto/qat/qat_adf/icp_qat_hw.h
index a08094f..ebe245f 100644
--- a/drivers/crypto/qat/qat_adf/icp_qat_hw.h
+++ b/drivers/crypto/qat/qat_adf/icp_qat_hw.h
@@ -237,6 +237,11 @@ enum icp_qat_hw_cipher_dir {
ICP_QAT_HW_CIPHER_DECRYPT = 1,
};
+enum icp_qat_hw_auth_op {
+ ICP_QAT_HW_AUTH_VERIFY = 0,
+ ICP_QAT_HW_AUTH_GENERATE = 1,
+};
+
enum icp_qat_hw_cipher_convert {
ICP_QAT_HW_CIPHER_NO_CONVERT = 0,
ICP_QAT_HW_CIPHER_KEY_CONVERT = 1,
diff --git a/drivers/crypto/qat/qat_adf/qat_algs.h b/drivers/crypto/qat/qat_adf/qat_algs.h
index 78a92f3..dcc0df5 100644
--- a/drivers/crypto/qat/qat_adf/qat_algs.h
+++ b/drivers/crypto/qat/qat_adf/qat_algs.h
@@ -98,6 +98,7 @@ struct qat_session {
enum icp_qat_hw_cipher_dir qat_dir;
enum icp_qat_hw_cipher_mode qat_mode;
enum icp_qat_hw_auth_algo qat_hash_alg;
+ enum icp_qat_hw_auth_op auth_op;
struct qat_alg_cd cd;
uint8_t *cd_cur_ptr;
phys_addr_t cd_paddr;
diff --git a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
index 0b66b37..8900668 100644
--- a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
+++ b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
@@ -625,11 +625,13 @@ int qat_alg_aead_session_create_content_desc_auth(struct qat_session *cdesc,
ICP_QAT_FW_LA_NO_RET_AUTH_RES);
ICP_QAT_FW_LA_CMP_AUTH_SET(header->serv_specif_flags,
ICP_QAT_FW_LA_CMP_AUTH_RES);
+ cdesc->auth_op = ICP_QAT_HW_AUTH_VERIFY;
} else {
ICP_QAT_FW_LA_RET_AUTH_SET(header->serv_specif_flags,
ICP_QAT_FW_LA_RET_AUTH_RES);
ICP_QAT_FW_LA_CMP_AUTH_SET(header->serv_specif_flags,
ICP_QAT_FW_LA_NO_CMP_AUTH_RES);
+ cdesc->auth_op = ICP_QAT_HW_AUTH_GENERATE;
}
/*
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index f8db12f..798cd98 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -951,6 +951,13 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
struct icp_qat_fw_la_cipher_req_params *cipher_param;
struct icp_qat_fw_la_auth_req_params *auth_param;
register struct icp_qat_fw_la_bulk_req *qat_req;
+ uint8_t do_auth = 0, do_cipher = 0;
+ uint32_t cipher_len = 0, cipher_ofs = 0;
+ uint32_t auth_len = 0, auth_ofs = 0;
+ uint32_t min_ofs = 0;
+ uint32_t digest_appended = 1;
+ uint64_t buf_start = 0;
+
#ifdef RTE_LIBRTE_PMD_QAT_DEBUG_TX
if (unlikely(op->type != RTE_CRYPTO_OP_TYPE_SYMMETRIC)) {
@@ -975,88 +982,173 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
qat_req = (struct icp_qat_fw_la_bulk_req *)out_msg;
rte_mov128((uint8_t *)qat_req, (const uint8_t *)&(ctx->fw_req));
qat_req->comn_mid.opaque_data = (uint64_t)(uintptr_t)op;
+ cipher_param = (void *)&qat_req->serv_specif_rqpars;
+ auth_param = (void *)((uint8_t *)cipher_param + sizeof(*cipher_param));
- qat_req->comn_mid.dst_length =
- qat_req->comn_mid.src_length =
- rte_pktmbuf_data_len(op->sym->m_src);
+ if (ctx->qat_cmd == ICP_QAT_FW_LA_CMD_HASH_CIPHER ||
+ ctx->qat_cmd == ICP_QAT_FW_LA_CMD_CIPHER_HASH) {
+ do_auth = 1;
+ do_cipher = 1;
+ } else if (ctx->qat_cmd == ICP_QAT_FW_LA_CMD_AUTH) {
+ do_auth = 1;
+ do_cipher = 0;
+ } else if (ctx->qat_cmd == ICP_QAT_FW_LA_CMD_CIPHER) {
+ do_auth = 0;
+ do_cipher = 1;
+ }
- qat_req->comn_mid.dest_data_addr =
- qat_req->comn_mid.src_data_addr =
- rte_pktmbuf_mtophys(op->sym->m_src);
+ if (do_cipher) {
- if (unlikely(op->sym->m_dst != NULL)) {
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys(op->sym->m_dst);
- qat_req->comn_mid.dst_length =
- rte_pktmbuf_data_len(op->sym->m_dst);
+ if (ctx->qat_cipher_alg ==
+ ICP_QAT_HW_CIPHER_ALGO_SNOW_3G_UEA2 ||
+ ctx->qat_cipher_alg == ICP_QAT_HW_CIPHER_ALGO_KASUMI) {
+
+ if (unlikely(
+ (cipher_param->cipher_length % BYTE_LENGTH != 0)
+ || (cipher_param->cipher_offset
+ % BYTE_LENGTH != 0))) {
+ PMD_DRV_LOG(ERR,
+ "SNOW3G/KASUMI in QAT PMD only supports byte aligned values");
+ op->status = RTE_CRYPTO_OP_STATUS_INVALID_ARGS;
+ return -EINVAL;
+ }
+ cipher_len = op->sym->cipher.data.length >> 3;
+ cipher_ofs = op->sym->cipher.data.offset >> 3;
+
+ } else {
+ cipher_len = op->sym->cipher.data.length;
+ cipher_ofs = op->sym->cipher.data.offset;
+ }
+
+ /* copy IV into request if it fits */
+ if (op->sym->cipher.iv.length && (op->sym->cipher.iv.length <=
+ sizeof(cipher_param->u.cipher_IV_array))) {
+ rte_memcpy(cipher_param->u.cipher_IV_array,
+ op->sym->cipher.iv.data,
+ op->sym->cipher.iv.length);
+ } else {
+ ICP_QAT_FW_LA_CIPH_IV_FLD_FLAG_SET(
+ qat_req->comn_hdr.serv_specif_flags,
+ ICP_QAT_FW_CIPH_IV_64BIT_PTR);
+ cipher_param->u.s.cipher_IV_ptr =
+ op->sym->cipher.iv.phys_addr;
+ }
+ min_ofs = cipher_ofs;
}
- cipher_param = (void *)&qat_req->serv_specif_rqpars;
- auth_param = (void *)((uint8_t *)cipher_param + sizeof(*cipher_param));
+ if (do_auth) {
- cipher_param->cipher_length = op->sym->cipher.data.length;
- cipher_param->cipher_offset = op->sym->cipher.data.offset;
- if (ctx->qat_cipher_alg == ICP_QAT_HW_CIPHER_ALGO_SNOW_3G_UEA2 ||
- ctx->qat_cipher_alg == ICP_QAT_HW_CIPHER_ALGO_KASUMI) {
- if (unlikely((cipher_param->cipher_length % BYTE_LENGTH != 0) ||
- (cipher_param->cipher_offset
- % BYTE_LENGTH != 0))) {
- PMD_DRV_LOG(ERR, " For SNOW 3G/KASUMI, QAT PMD only "
- "supports byte aligned values");
- op->status = RTE_CRYPTO_OP_STATUS_INVALID_ARGS;
- return -EINVAL;
+ if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_SNOW_3G_UIA2 ||
+ ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_KASUMI_F9) {
+ if (unlikely((auth_param->auth_off % BYTE_LENGTH != 0)
+ || (auth_param->auth_len % BYTE_LENGTH != 0))) {
+ PMD_DRV_LOG(ERR,
+ "For SNOW3G/KASUMI, QAT PMD only supports byte aligned values");
+ op->status = RTE_CRYPTO_OP_STATUS_INVALID_ARGS;
+ return -EINVAL;
+ }
+ auth_ofs = op->sym->auth.data.offset >> 3;
+ auth_len = op->sym->auth.data.length >> 3;
+
+ if (ctx->qat_hash_alg ==
+ ICP_QAT_HW_AUTH_ALGO_KASUMI_F9) {
+ if (do_cipher) {
+ auth_len = auth_len + auth_ofs + 1 -
+ ICP_QAT_HW_KASUMI_BLK_SZ;
+ auth_ofs = ICP_QAT_HW_KASUMI_BLK_SZ;
+ } else {
+ auth_len = auth_len + auth_ofs + 1;
+ auth_ofs = 0;
+ }
+ }
+
+ } else {
+ auth_ofs = op->sym->auth.data.offset;
+ auth_len = op->sym->auth.data.length;
+ }
+ min_ofs = auth_ofs;
+
+ if (op->sym->auth.digest.phys_addr) {
+ ICP_QAT_FW_LA_DIGEST_IN_BUFFER_SET(
+ qat_req->comn_hdr.serv_specif_flags,
+ ICP_QAT_FW_LA_NO_DIGEST_IN_BUFFER);
+ auth_param->auth_res_addr =
+ op->sym->auth.digest.phys_addr;
+ digest_appended = 0;
}
- cipher_param->cipher_length >>= 3;
- cipher_param->cipher_offset >>= 3;
+
+ auth_param->u1.aad_adr = op->sym->auth.aad.phys_addr;
+
}
- if (op->sym->cipher.iv.length && (op->sym->cipher.iv.length <=
- sizeof(cipher_param->u.cipher_IV_array))) {
- rte_memcpy(cipher_param->u.cipher_IV_array,
- op->sym->cipher.iv.data,
- op->sym->cipher.iv.length);
+ /* adjust for chain case */
+ if (do_cipher && do_auth)
+ min_ofs = cipher_ofs < auth_ofs ? cipher_ofs : auth_ofs;
+
+
+ /* Start DMA at nearest aligned address below min_ofs */
+ #define QAT_64_BTYE_ALIGN_MASK (~0x3f)
+ buf_start = rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs) &
+ QAT_64_BTYE_ALIGN_MASK;
+
+ if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src)
+ - rte_pktmbuf_headroom(op->sym->m_src)) > buf_start)) {
+ /* alignment has pushed addr ahead of start of mbuf
+ * so revert and take the performance hit
+ */
+ buf_start = rte_pktmbuf_mtophys(op->sym->m_src);
+ }
+
+ qat_req->comn_mid.dest_data_addr =
+ qat_req->comn_mid.src_data_addr = buf_start;
+
+ if (do_cipher) {
+ cipher_param->cipher_offset =
+ (uint32_t)rte_pktmbuf_mtophys_offset(
+ op->sym->m_src, cipher_ofs) - buf_start;
+ cipher_param->cipher_length = cipher_len;
} else {
- ICP_QAT_FW_LA_CIPH_IV_FLD_FLAG_SET(
- qat_req->comn_hdr.serv_specif_flags,
- ICP_QAT_FW_CIPH_IV_64BIT_PTR);
- cipher_param->u.s.cipher_IV_ptr = op->sym->cipher.iv.phys_addr;
+ cipher_param->cipher_offset = 0;
+ cipher_param->cipher_length = 0;
}
- if (op->sym->auth.digest.phys_addr) {
- ICP_QAT_FW_LA_DIGEST_IN_BUFFER_SET(
- qat_req->comn_hdr.serv_specif_flags,
- ICP_QAT_FW_LA_NO_DIGEST_IN_BUFFER);
- auth_param->auth_res_addr = op->sym->auth.digest.phys_addr;
+ if (do_auth) {
+ auth_param->auth_off = (uint32_t)rte_pktmbuf_mtophys_offset(
+ op->sym->m_src, auth_ofs) - buf_start;
+ auth_param->auth_len = auth_len;
+ } else {
+ auth_param->auth_off = 0;
+ auth_param->auth_len = 0;
}
- auth_param->auth_off = op->sym->auth.data.offset;
- auth_param->auth_len = op->sym->auth.data.length;
- if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_SNOW_3G_UIA2) {
- if (unlikely((auth_param->auth_off % BYTE_LENGTH != 0) ||
- (auth_param->auth_len % BYTE_LENGTH != 0))) {
- PMD_DRV_LOG(ERR, " For SNOW 3G, QAT PMD only "
- "supports byte aligned values");
- op->status = RTE_CRYPTO_OP_STATUS_INVALID_ARGS;
- return -EINVAL;
- }
- auth_param->auth_off >>= 3;
- auth_param->auth_len >>= 3;
+ qat_req->comn_mid.dst_length =
+ qat_req->comn_mid.src_length =
+ (cipher_param->cipher_offset + cipher_param->cipher_length)
+ > (auth_param->auth_off + auth_param->auth_len) ?
+ (cipher_param->cipher_offset + cipher_param->cipher_length)
+ : (auth_param->auth_off + auth_param->auth_len);
+
+ if (do_auth && digest_appended) {
+ if (ctx->auth_op == ICP_QAT_HW_AUTH_GENERATE)
+ qat_req->comn_mid.dst_length
+ += op->sym->auth.digest.length;
+ else
+ qat_req->comn_mid.src_length
+ += op->sym->auth.digest.length;
}
- if ((ctx->qat_cmd == ICP_QAT_FW_LA_CMD_HASH_CIPHER ||
- ctx->qat_cmd == ICP_QAT_FW_LA_CMD_CIPHER_HASH) &&
- ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_KASUMI_F9) {
- auth_param->auth_len = (auth_param->auth_len >> 3)
- + (auth_param->auth_off >> 3)
- + (BYTE_LENGTH >> 3)
- - 8;
- auth_param->auth_off = 8;
- } else if (ctx->qat_cmd == ICP_QAT_FW_LA_CMD_AUTH
- && ctx->qat_hash_alg ==
- ICP_QAT_HW_AUTH_ALGO_KASUMI_F9) {
- auth_param->auth_len = (auth_param->auth_len >> 3)
- + (auth_param->auth_off >> 3)
- + (BYTE_LENGTH >> 3);
- auth_param->auth_off = 0;
+
+ /* out-of-place operation (OOP) */
+ if (unlikely(op->sym->m_dst != NULL)) {
+
+ if (do_auth)
+ qat_req->comn_mid.dest_data_addr =
+ rte_pktmbuf_mtophys_offset(op->sym->m_dst,
+ auth_ofs)
+ - auth_param->auth_off;
+ else
+ qat_req->comn_mid.dest_data_addr =
+ rte_pktmbuf_mtophys_offset(op->sym->m_dst,
+ cipher_ofs)
+ - cipher_param->cipher_offset;
}
- auth_param->u1.aad_adr = op->sym->auth.aad.phys_addr;
if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128 ||
ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
@@ -1075,15 +1167,20 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
*/
qat_req->comn_mid.dest_data_addr =
qat_req->comn_mid.src_data_addr =
- op->sym->auth.aad.phys_addr;
+ op->sym->auth.aad.phys_addr;
+ qat_req->comn_mid.dst_length =
+ qat_req->comn_mid.src_length =
+ rte_pktmbuf_data_len(op->sym->m_src);
+ cipher_param->cipher_length = 0;
+ cipher_param->cipher_offset = 0;
auth_param->u1.aad_adr = 0;
auth_param->auth_len = op->sym->auth.aad.length;
+ auth_param->auth_off = op->sym->auth.data.offset;
auth_param->u2.aad_sz = 0;
-
}
-
}
+
#ifdef RTE_LIBRTE_PMD_QAT_DEBUG_TX
rte_hexdump(stdout, "qat_req:", qat_req,
sizeof(struct icp_qat_fw_la_bulk_req));
--
2.5.0
^ permalink raw reply related
* Re: [PATCH v4 11/17] eal/soc: add default scan for Soc devices
From: Shreyansh Jain @ 2016-10-24 12:08 UTC (permalink / raw)
To: Jan Viktorin
Cc: dev@dpdk.org, thomas.monjalon@6wind.com, david.marchand@6wind.com
In-Reply-To: <DB5PR0401MB20542DAB10891607EF5BDEA490D10@DB5PR0401MB2054.eurprd04.prod.outlook.com>
Hi Jan,
On Sunday 16 October 2016 12:42 PM, Shreyansh Jain wrote:
> Hi Jan,
>
>> -----Original Message-----
>> From: Jan Viktorin [mailto:viktorin@rehivetech.com]
>> Sent: Sunday, October 16, 2016 6:27 AM
>> To: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; david.marchand@6wind.com
>> Subject: Re: [PATCH v4 11/17] eal/soc: add default scan for Soc devices
>>
>> On Sat, 15 Oct 2016 19:15:02 +0530
>> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>
>>> From: Jan Viktorin <viktorin@rehivetech.com>
>>>
>>> Default implementation which scans the sysfs platform devices hierarchy..
>>> For each device, extract the ueven and convert into rte_soc_device.
>>>
>>> The information populated can then be used in probe to match against
>>> the drivers registered.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> [Shreyansh: restructure commit to be an optional implementation]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>
>> [...]
>>
>>> +
>>> +int
>>> +rte_eal_soc_scan(void)
>>
>> What about naming it rte_eal_soc_scan_default? This would underline the
>> fact that this function can be replaced.
>
> Yes, that would be in sync with match default. I will do it.
In v5 I have replaced the name with rte_eal_soc_platform_bus(). This is
long but it does exactly what the name states - scan for platform bus.
This is still a helper.
>
>>
>> Second, this is for the 7/17 patch:
>>
>> -/* register a driver */
>> void
>> rte_eal_soc_register(struct rte_soc_driver *driver)
>> {
>> + /* For a valid soc driver, match and scan function
>> + * should be provided.
>> + */
>> + RTE_VERIFY(driver != NULL);
>> + RTE_VERIFY(driver->match_fn != NULL);
>> + RTE_VERIFY(driver->scan_fn != NULL);
>>
>> What about setting the match_fn and scan_fn to default implementations if
>> they
>> are NULL? This would make the standard/default approach easier to use.
>>
>> TAILQ_INSERT_TAIL(&soc_driver_list, driver, next);
>> }
>
> I am not in favor of a forced default. What if user never intended it - it would lead to wrong scan being used and only intimation which can provided to user is a log.
> Selecting such functions should be a model of PMD - one which is enforced.
As mentioned before, I am not in favor of a 'default' implementation.
Thus, I would rather call these functions as 'helpers' rather than defaults.
[...]
-
Shreyansh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox