* [PATCH v13 0/7] add Tx preparation
From: Tomasz Kulasek @ 2016-12-13 17:41 UTC (permalink / raw)
To: dev
In-Reply-To: <1479922585-8640-1-git-send-email-tomaszx.kulasek@intel.com>
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_prepare) 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_prepare() 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_prepare" 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_prepare(port, 0, bufs, nb_pkts);
if (nb_prep < nb_pkts) {
printf("Tx prepare 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. */
v13 changes:
- added support for vmxnet3
- reworded help information for "csum txprep" command
- renamed RTE_ETHDEV_TX_PREPARE to RTE_ETHDEV_TX_PREPARE_NOOP to
better suit its purpose.
v12 changes:
- renamed API function from "rte_eth_tx_prep" to "rte_eth_tx_prepare"
(to be not confused with "prepend")
- changed "rte_phdr_cksum_fix" to "rte_net_intel_cksum_prepare"
- added "csum txprep (on|off)" command to the csum engine allowing to
select txprep path for packet processing
v11 changed:
- updated comments
- added information to the API description about packet data
requirements/limitations.
v10 changes:
- moved drivers tx calback check in rte_eth_tx_prep after queue_id check
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
Ananyev, Konstantin (1):
vmxnet3: add Tx preparation
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/cmdline.c | 49 ++++++++++++
app/test-pmd/csumonly.c | 33 ++++++--
app/test-pmd/testpmd.c | 5 ++
app/test-pmd/testpmd.h | 2 +
config/common_base | 9 +++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 13 +++
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 | 74 ++++++++++++++++-
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 | 56 +++++++++++++
drivers/net/ixgbe/ixgbe_rxtx.h | 2 +
drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 +
drivers/net/vmxnet3/vmxnet3_ethdev.h | 2 +
drivers/net/vmxnet3/vmxnet3_rxtx.c | 57 +++++++++++++
lib/librte_ether/rte_ethdev.h | 115 +++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf.h | 64 +++++++++++++++
lib/librte_net/rte_net.h | 85 ++++++++++++++++++++
27 files changed, 757 insertions(+), 13 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH 06/13] null: fake PMD capabilities
From: Ferruh Yigit @ 2016-12-13 17:31 UTC (permalink / raw)
To: Ananyev, Konstantin, Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E754B@irsmsx105.ger.corp.intel.com>
On 12/13/2016 5:12 PM, Ananyev, Konstantin wrote:
>
>
>> -----Original Message-----
>> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
>> Sent: Tuesday, December 13, 2016 2:58 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
>>
>> On Tue, Dec 13, 2016 at 02:37:34PM +0000, Ananyev, Konstantin wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
>>>> Sent: Tuesday, December 13, 2016 2:27 PM
>>>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
>>>>
>>>> On Tue, Dec 13, 2016 at 10:48:32AM +0000, Ananyev, Konstantin wrote:
>>>>>> -----Original Message-----
>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
>>>>>> Sent: Tuesday, December 13, 2016 1:08 AM
>>>>>> To: dev@dpdk.org
>>>>>> Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
>>>>>>
>>>>>> From: Paweł Małachowski <pawel.malachowski@atendesoftware.pl>
>>>>>>
>>>>>> Thanks to that change we can use Null PMD for testing purposes.
>>>>>>
>>>>>> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
>>>>>> ---
>>>>>> drivers/net/null/rte_eth_null.c | 13 ++++++++++++-
>>>>>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
>>>>>> index 836d982..f32ba2a 100644
>>>>>> --- a/drivers/net/null/rte_eth_null.c
>>>>>> +++ b/drivers/net/null/rte_eth_null.c
>>>>>> @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +static void
>>>>>> +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
>>>>>> +
>>>>>>
>>>>>> static void
>>>>>> eth_dev_info(struct rte_eth_dev *dev,
>>>>>> @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
>>>>>> dev_info->pci_dev = NULL;
>>>>>> dev_info->reta_size = internals->reta_size;
>>>>>> dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
>>>>>> + /* We hereby declare we can RX-offload VLAN-s out of thin air and update checksums and VLANs before sinking
>> packets in
>>>>>> /dev/null */
>>>>>> + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>>>>>> + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT | DEV_TX_OFFLOAD_IPV4_CKSUM;
>>>>>
>>>>> Hmm, how could it be supported if all that null PMD does on TX - just free the packets?
>>>>> Same question for RX.
>>>>
>>>> You could imagine, that before dropping the packet you insert the tag
>>>> and calculate the checksum. The observed effect will be the same.
>>>>
>>>> On RX this always indicates lack of VLAN tag. So whether the offload
>>>> is enabled or not it doesn't matter.
>>>
>>> Of course user can imagine whatever he likes, but what the point to advertise these capabilities,
>>> when the PMD clearly doesn't have them?
>>> Again, why these particular ones?
>>
>> Hmm. I guess we can expand the set. Those were the ones we actually use
>> (depend on) for testing our code.
>>
>> This allows to use null PMD for testing in place of real network interface
>> with those offloads.
>
> As I can see on TX null pmd would just drop the packets, right?
> So the only thing you might check, as I understand, did upper layer code
> setup ol_flags and other mbuf fields properly depending on advertised capabilities
> (probably via sme special tx_callback installed or so).
> Is that correct?
> That's ok, I suppose, but if tomorrow you (or someone else) would like to test
> with different RX/TX offloads?
> Shouldn't be advertised offload capabilities be configurable at device creation/attach time
> somehow?
This sounds good. Getting offload capabilities on runtime as devargs.
> Or at least just advertise all possible ones then?
>
> Konstantin
>
>> This patch is to keep users from having to place if's
>> around their code just to support test scenarios with null PMD.
>>
>> Best Regards,
>> Michał Mirosław
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Ananyev, Konstantin @ 2016-12-13 17:27 UTC (permalink / raw)
To: Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <20161213161409.ekbagsze5pcy2ppl@rere.qmqm.pl>
> -----Original Message-----
> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> Sent: Tuesday, December 13, 2016 4:14 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
>
> On Tue, Dec 13, 2016 at 03:13:42PM +0000, Ananyev, Konstantin wrote:
> [...]
> > > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > > > > > >
> > > > > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > > > > ---
> > > > > > > lib/librte_acl/rte_acl.c | 3 +--
> > > > > > > lib/librte_acl/rte_acl.h | 2 --
> > > > > > > lib/librte_table/rte_table_acl.c | 2 +-
> > > > > > > 3 files changed, 2 insertions(+), 5 deletions(-)
> > > > > > >
> > > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > > > > > > index 8b7e92c..d1f40be 100644
> > > > > > > --- a/lib/librte_acl/rte_acl.c
> > > > > > > +++ b/lib/librte_acl/rte_acl.c
> > > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > > > > > > rd->category_mask) == 0 ||
> > > > > > > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > > > > > > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > > > > > > return -EINVAL;
> > > > > > > return 0;
> > > > > > > }
> > > > > >
> > > > > > I am not sure, how it supposed to work properly?
> > > > > > Zero value is reserved and ifnicates that no match were found for that input.
> > > > >
> > > > > This is actually in use by us. In our use we don't need to differentiate
> > > > > matching a rule with zero verdict vs not matching a rule at all. I also
> > > > > have a patch that changes the value returned in non-matching case, but
> > > > > it's in "dirty hack" state, as of yet.
> > > >
> > > > With that chane rte_acl_classify() might produce invalid results.
> > > > Even if you don't need it (I still don't understand how) , it doesn't mean other people
> > > > don't need it either and it is ok to change it.
> > > >
> > > > >
> > > > > The ACL code does not treat zero userdata specially, so this is only
> > > > > a policy choice and as such would be better to be made by the user.
> > > >
> > > > I believe it does.
> > > > userdata==0 is a reserved value.
> > > > When rte_acl_clasify() returns 0 for that particular input, it means 'no matches were found'.
> > >
> > > Dear Konstantin,
> > >
> > > Can you describe how the ACL code treats zero specially? I could not find
> > > anything, really. The only thing I found is that iff I use zero userdata
> > > in a rule I won't be able to differentiate a case where it matched from
> > > a case where no rule matched.
> >
> > Yes, that's what I am talking about.
> >
> > > If I all my rules have non-zero userdata,
> > > then this patch changes nothing.
> >
> > Ok, then why do you remove a code that does checking for invalid userdata==0?
> > That supposed to prevent user to setup invalid value by mistake.
> >
> > But if I have a table where 0 means drop
> > > (default-drop policy) then being able to use zero userdata in DROP rules
> > > makes the ACLs just that more useful.
> >
> > Ok, and what prevents you from do +1 to your policy values before
> > you insert it into the ACL table and -1 after you retrieved it via rte_acl_classify()?
>
> The check is enforcing an assumption that all users want to distinguish
> the cases whether any rule matched and whether no rules matched. Not all
> users do, hence the assumption is invalid and this patch removes it.
The check is based on the assumption that users might need to distinguish
the situation when no rules were matched.
To support that we need a reserved userdata value, which would mean
NO_MATCH.
>From what I heard, most users do need this ability, those who don't
can easily overcome it.
>
> Yes, people can work around it by loosing 1 of 2^32 useful values and
> convoluting their code.
Yes, one of 2^32 values is reserved.
Any reason why (2^32 - 1) values might not be enough?
>
> You seem to argue that 0 is somehow an invalid value, but I can't find
> anything in the ACL that would require it to be so. Could you point me
> to the code in DPDK where this actually matters?
It was a while, when I looked into ACL code in details, but as remember
that's the only reason: we need some value to be reserved as NO_MATCH.
Let say in build_trie() we set results to zero for rules with unused categories:
for (m = context->cfg.num_categories; 0 != m--; ) {
if (rule->f->data.category_mask & (1 << m)) {
end->mrt->results[m] = rule->f->data.userdata;
end->mrt->priority[m] = rule->f->data.priority;
} else {
end->mrt->results[m] = 0;
end->mrt->priority[m] = 0;
}
}
Konstantin
^ permalink raw reply
* Re: [PATCH 06/13] null: fake PMD capabilities
From: Ananyev, Konstantin @ 2016-12-13 17:12 UTC (permalink / raw)
To: Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <20161213145742.hxvtr63anhy7ai33@rere.qmqm.pl>
> -----Original Message-----
> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> Sent: Tuesday, December 13, 2016 2:58 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
>
> On Tue, Dec 13, 2016 at 02:37:34PM +0000, Ananyev, Konstantin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> > > Sent: Tuesday, December 13, 2016 2:27 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> > >
> > > On Tue, Dec 13, 2016 at 10:48:32AM +0000, Ananyev, Konstantin wrote:
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > > > To: dev@dpdk.org
> > > > > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> > > > >
> > > > > From: Paweł Małachowski <pawel.malachowski@atendesoftware.pl>
> > > > >
> > > > > Thanks to that change we can use Null PMD for testing purposes.
> > > > >
> > > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > > ---
> > > > > drivers/net/null/rte_eth_null.c | 13 ++++++++++++-
> > > > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> > > > > index 836d982..f32ba2a 100644
> > > > > --- a/drivers/net/null/rte_eth_null.c
> > > > > +++ b/drivers/net/null/rte_eth_null.c
> > > > > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static void
> > > > > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > > > > +
> > > > >
> > > > > static void
> > > > > eth_dev_info(struct rte_eth_dev *dev,
> > > > > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > > > > dev_info->pci_dev = NULL;
> > > > > dev_info->reta_size = internals->reta_size;
> > > > > dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
> > > > > + /* We hereby declare we can RX-offload VLAN-s out of thin air and update checksums and VLANs before sinking
> packets in
> > > > > /dev/null */
> > > > > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > > > > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT | DEV_TX_OFFLOAD_IPV4_CKSUM;
> > > >
> > > > Hmm, how could it be supported if all that null PMD does on TX - just free the packets?
> > > > Same question for RX.
> > >
> > > You could imagine, that before dropping the packet you insert the tag
> > > and calculate the checksum. The observed effect will be the same.
> > >
> > > On RX this always indicates lack of VLAN tag. So whether the offload
> > > is enabled or not it doesn't matter.
> >
> > Of course user can imagine whatever he likes, but what the point to advertise these capabilities,
> > when the PMD clearly doesn't have them?
> > Again, why these particular ones?
>
> Hmm. I guess we can expand the set. Those were the ones we actually use
> (depend on) for testing our code.
>
> This allows to use null PMD for testing in place of real network interface
> with those offloads.
As I can see on TX null pmd would just drop the packets, right?
So the only thing you might check, as I understand, did upper layer code
setup ol_flags and other mbuf fields properly depending on advertised capabilities
(probably via sme special tx_callback installed or so).
Is that correct?
That's ok, I suppose, but if tomorrow you (or someone else) would like to test
with different RX/TX offloads?
Shouldn't be advertised offload capabilities be configurable at device creation/attach time
somehow?
Or at least just advertise all possible ones then?
Konstantin
>This patch is to keep users from having to place if's
> around their code just to support test scenarios with null PMD.
>
> Best Regards,
> Michał Mirosław
^ permalink raw reply
* Re: [PATCH 20/31] app/testpmd: use VFD APIs on i40e
From: Ferruh Yigit @ 2016-12-13 16:57 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark), Bernard Iremonger
In-Reply-To: <1480637533-37425-21-git-send-email-wenzhuo.lu@intel.com>
On 12/2/2016 12:12 AM, Wenzhuo Lu wrote:
> The new VF Daemon (VFD) APIs is implemented on i40e. Change
> testpmd code to use them, inlcuding VF MAC anti-spoofing,
> VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN
> insert.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
> app/test-pmd/cmdline.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 85 insertions(+), 7 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 63b55dc..1284d6c 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
<...>
> @@ -11059,8 +11109,20 @@ struct cmd_vf_vlan_insert_result {
> {
> struct cmd_vf_vlan_insert_result *res = parsed_result;
> int ret;
> + struct rte_eth_dev_info dev_info;
> +
> + memset(&dev_info, 0, sizeof(dev_info));
> + rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> + if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> + ret = rte_pmd_ixgbe_set_vf_vlan_insert(res->port_id, res->vf_id,
> + res->vlan_id);
> + else if (strstr(dev_info.driver_name, "i40e") != NULL)
> + ret = rte_pmd_i40e_set_vf_vlan_insert(res->port_id, res->vf_id,
> + res->vlan_id);
This code is within "#ifdef RTE_LIBRTE_IXGBE_PMD", so i40e check can
fail if IXGBE_PMD disabled, need to update surrounding ifdef.
Same for rest.
<...>
^ permalink raw reply
* [PATCH v2] SDK: Add scripts to initialize DPDK runtime
From: Luca Boccassi @ 2016-12-13 16:47 UTC (permalink / raw)
To: dev; +Cc: stefan.bader, bruce.richardson, Christian Ehrhardt, Luca Boccassi
In-Reply-To: <1481570642-15138-1-git-send-email-lboccass@brocade.com>
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
A tools/init directory is added with dpdk-init, a script that can be
used to initialize a DPDK runtime environment. 2 config files with
default options, dpdk.conf and interfaces, are provided as well
together with a SysV init script and a systemd service unit.
v2: relicensed dpdk-init.in from GPL3 to BSD-3-clause with authors'
permission
Signed-off-by: Luca Boccassi <lboccass@brocade.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
mk/rte.sdkinstall.mk | 21 ++++
tools/init/dpdk-init.in | 274 +++++++++++++++++++++++++++++++++++++++++++++
tools/init/dpdk.conf | 60 ++++++++++
tools/init/dpdk.init.in | 57 ++++++++++
tools/init/dpdk.service.in | 12 ++
tools/init/interfaces | 16 +++
6 files changed, 440 insertions(+)
create mode 100755 tools/init/dpdk-init.in
create mode 100644 tools/init/dpdk.conf
create mode 100755 tools/init/dpdk.init.in
create mode 100644 tools/init/dpdk.service.in
create mode 100644 tools/init/interfaces
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 7b0d8b5..a3a5a9a 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -69,6 +69,14 @@ datadir ?= $(datarootdir)/dpdk
mandir ?= $(datarootdir)/man
sdkdir ?= $(datadir)
targetdir ?= $(datadir)/$(RTE_TARGET)
+# If pkgconfig or systemd.pc are not available fall back to most likely default
+ifeq ($(shell pkg-config systemd; echo $$?), 0)
+systemduserunitdir ?= $(shell pkg-config --variable=systemdsystemunitdir systemd)
+else
+systemduserunitdir ?= /lib/systemd/system
+endif
+initdir ?= /etc/init.d
+configdir ?= /etc/dpdk
# The install directories may be staged in DESTDIR
@@ -162,6 +170,19 @@ install-sdk:
$(Q)cp -a $O/app/dpdk-pmdinfogen $(DESTDIR)$(targetdir)/app
$(Q)$(call rte_symlink, $(DESTDIR)$(includedir), $(DESTDIR)$(targetdir)/include)
$(Q)$(call rte_symlink, $(DESTDIR)$(libdir), $(DESTDIR)$(targetdir)/lib)
+ $(Q)$(call rte_mkdir, $(DESTDIR)$(initdir))
+ $(Q)sed -e "s|@@configdir@@|$(configdir)|g" -e "s|@@sbindir@@|$(sbindir)|g" \
+ $(RTE_SDK)/tools/init/dpdk.init.in > $(DESTDIR)$(initdir)/dpdk
+ $(Q)chmod +x $(DESTDIR)$(initdir)/dpdk
+ $(Q)$(call rte_mkdir, $(DESTDIR)$(systemduserunitdir))
+ $(Q)sed "s|@@sbindir@@|$(sbindir)|g" $(RTE_SDK)/tools/init/dpdk.service.in > \
+ $(DESTDIR)$(systemduserunitdir)/dpdk.service
+ $(Q)$(call rte_mkdir, $(DESTDIR)$(configdir))
+ $(Q)cp -a $(RTE_SDK)/tools/init/dpdk.conf $(DESTDIR)$(configdir)
+ $(Q)cp -a $(RTE_SDK)/tools/init/interfaces $(DESTDIR)$(configdir)
+ $(Q)sed -e "s|@@configdir@@|$(configdir)|g" -e "s|@@sbindir@@|$(sbindir)|g" \
+ $(RTE_SDK)/tools/init/dpdk-init.in > $(DESTDIR)$(sbindir)/dpdk-init
+ $(Q)chmod +x $(DESTDIR)$(sbindir)/dpdk-init
install-doc:
ifneq ($(wildcard $O/doc/html),)
diff --git a/tools/init/dpdk-init.in b/tools/init/dpdk-init.in
new file mode 100755
index 0000000..a1a44f7
--- /dev/null
+++ b/tools/init/dpdk-init.in
@@ -0,0 +1,274 @@
+#!/bin/sh
+#
+# dpdk-init: startup script to initialize a dpdk runtime environment
+#
+# Autor: Stefan Bader <stefan.bader@canonical.com>
+# Autor: Christian Ehrhardt <christian.ehrhardt@canonical.com>
+#
+# BSD LICENSE
+#
+# Copyright(c) 2015-2016 Canonical Ltd. All rights reserved.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Intel Corporation nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+set -e
+
+DPDK_BIND="@@sbindir@@/dpdk-devbind"
+DPDK_INTERF="@@configdir@@/interfaces"
+DPDK_CONF="@@configdir@@/dpdk.conf"
+
+
+# pagesize supports [G|g]/[M|m]/[K|k]
+get_kbytes() {
+ local unit
+ local num
+ unit=$(echo "${1}" | sed 's/[0-9]*//g')
+ num=$(echo "${1}" | sed 's/[^0-9]*//g')
+ case ${unit} in
+ *g | *G)
+ echo $((num*1024*1024))
+ ;;
+ *m | *M)
+ echo $((num*1024))
+ ;;
+ *k | *K)
+ echo $((num))
+ ;;
+ *)
+ echo $((num/1024))
+ ;;
+ esac
+}
+
+get_default_hpgsz() {
+ default_hpgsz=$(grep "Hugepagesize:" /proc/meminfo \
+ | sed 's/^Hugepagesize:\s*//g' | sed 's/\s*kB$//g')
+ echo "${default_hpgsz}"
+}
+
+get_hugetlbfs_mountpoint() {
+ local requested_hpgsz
+ local mp_hpgsz
+ requested_hpgsz=$(get_kbytes "${1}")
+
+ grep hugetlbfs /proc/mounts | while read \
+ mntfrom mntpoint mntfstype mntopt mntdump mntfsck; do
+
+ # check if the current muntpoint is of the requested huge page size
+ case ${mntopt} in
+ *pagesize=*)
+ mp_hpgsz=$(echo "${mntopt}" | sed 's/.*pagesize=//g' | sed 's/,.*//g')
+ mp_hpgsz=$(get_kbytes "${mp_hpgsz}")
+ ;;
+ *)
+ mp_hpgsz=$(get_default_hpgsz)
+ ;;
+ esac
+ if [ "${requested_hpgsz}" -eq "${mp_hpgsz}" ]; then
+ echo "${mntpoint}"
+ return
+ fi
+ done
+}
+
+_mount_hugetlbfs() {
+ local MNT="/dev/hugepages"
+ local MNTOPTS=""
+ local requested_hpgsz
+ local default_hpgsz
+ requested_hpgsz=$(get_kbytes "${1}")
+ default_hpgsz=$(get_default_hpgsz)
+
+ # kernel might not support the requested size
+ if [ ! -d "/sys/kernel/mm/hugepages/hugepages-${requested_hpgsz}kB" ]; then
+ echo "WARNING: requested page size of ${requested_hpgsz}kB " \
+ "not supported by the kernel"
+ return 0
+ fi
+
+ # special case if this is not the default huge page size
+ if [ "${requested_hpgsz}" -ne "${default_hpgsz}" ]; then
+ MNT="${MNT}-${requested_hpgsz}"
+ MNTOPTS="pagesize=${requested_hpgsz}K"
+ fi
+
+ if [ ! -e "${MNT}" ]; then
+ mkdir "${MNT}"
+ if [ $? -ne 0 ]; then
+ echo "Could not create directory ${MNT}!" >&2
+ return 1
+ fi
+ fi
+ mount -thugetlbfs hugetlbfs "${MNT}" -o "${MNTOPTS}"
+ return $?
+}
+
+#
+# The DPDK library will use the first mounted instance it finds for a given
+# page size. so if there is already one for a given size there is no need to
+# create another for the same huge page size.
+#
+mount_hugetlbfs() {
+ if [ ! -r "$DPDK_CONF" ]; then
+ return 1
+ fi
+ . "$DPDK_CONF"
+
+ # if a page size is requested, there has to be a mountpoint for that size
+ if [ -n "${NR_2M_PAGES}" -a -z "$(get_hugetlbfs_mountpoint '2M')" ]; then
+ _mount_hugetlbfs 2M
+ fi
+ if [ -n "${NR_16M_PAGES}" -a -z "$(get_hugetlbfs_mountpoint '16M')" ]; then
+ _mount_hugetlbfs 16M
+ fi
+ if [ -n "${NR_1G_PAGES}" -a -z "$(get_hugetlbfs_mountpoint '1G')" ]; then
+ _mount_hugetlbfs 1G
+ fi
+}
+
+_setup_hugepages() {
+ MMDIR="/sys/kernel/mm/hugepages/${1}"
+ PAGES=${2}
+
+ if [ "$PAGES" != "" ]; then
+ if [ "$PAGES" -gt 0 ]; then
+ if [ -d "$MMDIR" -a -w "$MMDIR/nr_hugepages" ]; then
+ # increases the chance to allocate enough huge pages
+ # configurable, since it comes at a perf penality
+ if [ "$DROPCACHE_BEFORE_HP_ALLOC" = "1" ]; then
+ echo 3 > /proc/sys/vm/drop_caches
+ fi
+
+ echo "$PAGES" > "$MMDIR/nr_hugepages"
+
+ GOTPAGES=$(cat "$MMDIR/nr_hugepages")
+ if [ "$GOTPAGES" -lt "$PAGES" ]; then
+ echo "WARNING: could not allocate $PAGES at " \
+ "$MMDIR/nr_hugepages (only got $GOTPAGES)."
+ fi
+ else
+ echo "WARNING: $MMDIR/nr_hugepages not found/writable"
+ fi
+ fi
+ fi
+}
+
+#
+# Reserve a certain amount of hugepages (defined in /etc/dpdk.conf)
+#
+setup_hugepages() {
+ if [ ! -r "$DPDK_CONF" ]; then
+ return 1
+ fi
+ . "$DPDK_CONF"
+
+ _setup_hugepages "hugepages-2048kB" "$NR_2M_PAGES"
+ _setup_hugepages "hugepages-16384kB" "$NR_16M_PAGES"
+ _setup_hugepages "hugepages-1048576kB" "$NR_1G_PAGES"
+
+ # dpdk uses 2*#hugepages mappings, increase for huge systems LP #1507921
+ if [ -d /sys/kernel/mm/hugepages ]; then
+ max_map_count=$(awk -v pad=65530 '{tot+=$1}END{print tot*2+pad}' \
+ /sys/kernel/mm/hugepages/hugepages-*/nr_hugepages)
+ sysctl -q vm.max_map_count="${max_map_count:-65530}"
+ fi
+
+ return 0
+}
+
+#
+# Allow NICs to be automatically bound to DPDK compatible drivers on boot.
+#
+bind_interfaces() {
+ if [ ! -r "$DPDK_INTERF" ]; then
+ return 0
+ fi
+ grep -v '^[ \t]*#' "$DPDK_INTERF" | while read BUS ID MOD; do
+ if [ "$BUS" = "" -o "$ID" = "" -o "$MOD" = "" ]; then
+ echo "WARNING: incomplete spec in $DPDK_INTERF" \
+ " - BUS '$BUS' ID '$ID' MOD '$MOD'"
+ continue
+ fi
+ if [ "$BUS" != "pci" ]; then
+ echo "WARNING: incompatible bus '$BUS' in $DPDK_INTERF"
+ continue
+ fi
+
+ SYSFSPATH="/sys/bus/$BUS/devices/$ID"
+ if [ ! -e "$SYSFSPATH" ]; then
+ echo "WARNING: invalid pci ID '$ID' in $DPDK_INTERF" \
+ " - '$SYSFSPATH' does not exist"
+ continue
+ fi
+ if [ -L "$SYSFSPATH/driver" ]; then
+ CUR=$(readlink "$SYSFSPATH/driver")
+ CUR=$(basename "$CUR")
+ else
+ # device existing, but currently unregistered
+ CUR=""
+ fi
+ if [ "$MOD" != "$CUR" ]; then
+ modprobe -q "$MOD" || true
+ # cloud img have no linux-image-extra initially (uip_pci_generic)
+ # so check if the module is available (loadable/built in)
+ if [ -e "/sys/bus/pci/drivers/${MOD}" ]; then
+ echo "Reassigning pci:$ID to $MOD"
+ $DPDK_BIND -b "$MOD" "$ID"
+ else
+ echo "Warning: failed assigning pci:$ID," \
+ " module $MOD not available"
+ fi
+ else
+ echo "pci:$ID already assigned to $MOD"
+ fi
+ done
+}
+
+
+
+case "$1" in
+start)
+ mount_hugetlbfs
+ setup_hugepages
+ bind_interfaces
+ ;;
+stop)
+ ;;
+reload|force-reload)
+ setup_hugepages
+ bind_interfaces
+ ;;
+status)
+ $DPDK_BIND --status
+ ;;
+*)
+ echo "Usage: $0 {start|stop|reload|force-reload|status}"
+ exit 1
+ ;;
+esac
+
diff --git a/tools/init/dpdk.conf b/tools/init/dpdk.conf
new file mode 100644
index 0000000..a5aea86
--- /dev/null
+++ b/tools/init/dpdk.conf
@@ -0,0 +1,60 @@
+#
+# The number of 2M hugepages to reserve on system boot
+#
+# Default is 0
+# To e.g. let it reserve 128M via 64x 2M Hugepages set:
+# NR_2M_PAGES=64
+
+#
+# The number of 1G hugepages to reserve on system boot
+#
+# Default is 0
+# To e.g. let it reserve 2G via 2x 1G Hugepages set:
+# NR_1G_PAGES=2
+
+# The number of 16M hugepages to reserve, supported e.g. on ppc64el
+#
+# Default is 0
+# To e.g. let it reserve 512M via 32x 16M Hugepages set:
+# NR_16M_PAGES=32
+
+#
+# Dropping slab and pagecache can help to successfully allocate hugepages,
+# especially later in the lifecycle of a system.
+# This comes at the cost of loosing all slab and pagecache on (re)start
+# of the dpdk service - therefore the default is off.
+#
+# Default is 0
+# Set to 1 to enable it
+#DROPCACHE_BEFORE_HP_ALLOC=0
+
+# The DPDK library will use the first mounted hugetlbfs.
+# The init scripts try to ensure there is at least one default hugetlbfs
+# mountpoint on start.
+# If you have multiple hugetlbfs mountpoints for a complex (e.g. specific numa
+# policies) setup it should be controlled by the admin instead of this init
+# script. In that case specific mountpoints can be provided as parameters to
+# the DPDK library.
+
+# Hardware may support other granularities of hugepages (like 4M). But the
+# larger the hugepages the earlier those should be allocated.
+# Note: the dpdk init scripts will report warnings, but not fail if they could
+# not allocate the requested amount of hugepages.
+# The more or the larger the hugepages to be allocated are, the more it is
+# recommended to do the reservation as kernel commandline arguments.
+# To do so edit /etc/default/grub: GRUB_CMDLINE_LINUX_DEFAULT
+# and add [hugepagesz=xx] hugepages=yy ...
+#
+# Kernel commandline config:
+# hugepagesz sets the size for the next hugepages reservation (default 2M)
+# hugepages reserves the given number of hugepages of the size set before
+#
+# After modifying /etc/default/grub, the command "update-grub" has to be
+# run in order to re-generate the grub config files. The new values will
+# be used after next reboot.
+#
+# example:
+# GRUB_CMDLINE_LINUX_DEFAULT="... hugepages=16 hugepagesz=1G hugepages=2"
+#
+# If the system supports it, this will reserve 16x 2M pages and 2x 1G pages.
+#
diff --git a/tools/init/dpdk.init.in b/tools/init/dpdk.init.in
new file mode 100755
index 0000000..1e26450
--- /dev/null
+++ b/tools/init/dpdk.init.in
@@ -0,0 +1,57 @@
+#!/bin/sh
+
+### BEGIN INIT INFO
+# Provides: dpdk
+# Required-Start: $remote_fs $local_fs
+# Required-Stop: $remote_fs $local_fs
+# Default-Start: S
+# Default-Stop: 0 1 6
+# Short-Description: start dpdk runtime environment
+### END INIT INFO
+
+set -e
+
+PATH="/sbin:/bin:/usr/bin"
+
+[ -d @@configdir@@ ] || exit 0
+
+# Define LSB log_* functions.
+# Depend on lsb-base (>= 3.2-14) to ensure that this file is present
+# and status_of_proc is working.
+. /lib/lsb/init-functions
+
+error=0
+case "$1" in
+start)
+ log_action_begin_msg "Starting DPDK environment" "dpdk"
+ output=$(@@sbindir@@/dpdk-init start 2>&1) || error="$?"
+ if [ ! -z "$output" ]; then
+ echo "$output" | while read line; do
+ log_action_cont_msg "$line"
+ done
+ fi
+ log_action_end_msg $error
+ exit $error
+ ;;
+stop)
+ ;;
+restart|force-reload)
+ ;;
+status)
+ output=$(@@sbindir@@/dpdk-init --status 2>&1) || error="$?"
+ if [ ! -z "$output" ]; then
+ echo "$output" | while read line; do
+ log_action_cont_msg "$line"
+ done
+ fi
+ log_action_end_msg $error
+ exit $error
+ ;;
+*)
+ echo "Usage: $0 {start|stop|restart|force-reload|status}"
+ exit 1
+ ;;
+esac
+
+exit 0
+
diff --git a/tools/init/dpdk.service.in b/tools/init/dpdk.service.in
new file mode 100644
index 0000000..1968081
--- /dev/null
+++ b/tools/init/dpdk.service.in
@@ -0,0 +1,12 @@
+[Unit]
+Description=DPDK runtime environment
+DefaultDependencies=false
+After=network-pre.target local-fs.target
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=@@sbindir@@/dpdk-init start
+
+[Install]
+WantedBy=multi-user.target
diff --git a/tools/init/interfaces b/tools/init/interfaces
new file mode 100644
index 0000000..73c3fca
--- /dev/null
+++ b/tools/init/interfaces
@@ -0,0 +1,16 @@
+#
+# <bus> Currently only "pci" is supported
+# <id> Device ID on the specified bus
+# <driver> Driver to bind against (vfio-pci, uio_pci_generic, igb_uio or
+# rte_kni)
+#
+# Be aware that the two dpdk compatible drivers uio_pci_generic and vfio-pci are
+# part of linux-image-extra-<VERSION> package on Debian-based distributions.
+# This package is not always installed by default - for example in cloud-images.
+# So please install it in case you run into missing module issues.
+#
+# <bus> <id> <driver>
+# pci 0000:04:00.0 vfio-pci
+# pci 0000:04:00.1 uio_pci_generic
+# pci 0000:05:00.0 igb_uio
+# pci 0000:06:00.0 rte_kni
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 04/13] acl: allow zero verdict
From: Michal Miroslaw @ 2016-12-13 16:43 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <20161213161409.ekbagsze5pcy2ppl@rere.qmqm.pl>
On Tue, Dec 13, 2016 at 05:14:09PM +0100, Michal Miroslaw wrote:
> On Tue, Dec 13, 2016 at 03:13:42PM +0000, Ananyev, Konstantin wrote:
[...]
> > > Dear Konstantin,
> > >
> > > Can you describe how the ACL code treats zero specially? I could not find
> > > anything, really. The only thing I found is that iff I use zero userdata
> > > in a rule I won't be able to differentiate a case where it matched from
> > > a case where no rule matched.
> >
> > Yes, that's what I am talking about.
> >
> > > If I all my rules have non-zero userdata,
> > > then this patch changes nothing.
> >
> > Ok, then why do you remove a code that does checking for invalid userdata==0?
> > That supposed to prevent user to setup invalid value by mistake.
> >
> > But if I have a table where 0 means drop
> > > (default-drop policy) then being able to use zero userdata in DROP rules
> > > makes the ACLs just that more useful.
> >
> > Ok, and what prevents you from do +1 to your policy values before
> > you insert it into the ACL table and -1 after you retrieved it via rte_acl_classify()?
>
> The check is enforcing an assumption that all users want to distinguish
> the cases whether any rule matched and whether no rules matched. Not all
> users do, hence the assumption is invalid and this patch removes it.
>
> Yes, people can work around it by loosing 1 of 2^32 useful values and
> convoluting their code.
>
> You seem to argue that 0 is somehow an invalid value, but I can't find
> anything in the ACL that would require it to be so. Could you point me
> to the code in DPDK where this actually matters?
I just noticed that it's probably you who wrote most of the ACLs code,
so I guest you're the right person to ask the question above.
Nice work, BTW. :-)
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH v2 09/13] PMD/af_packet: guard against buffer overruns in TX path
From: John W. Linville @ 2016-12-13 16:06 UTC (permalink / raw)
To: Michał Mirosław; +Cc: dev
In-Reply-To: <9c988a6bcc6c4179685cce5ae0560f850e7fad2e.1481592081.git.mirq-linux@rere.qmqm.pl>
On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
Acked-by: John W. Linville <linville@tuxdriver.com>
> ---
> drivers/net/af_packet/rte_eth_af_packet.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index 5599e02..fc2dc4a 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -83,6 +83,7 @@ struct pkt_rx_queue {
>
> struct pkt_tx_queue {
> int sockfd;
> + unsigned int frame_data_size;
>
> struct iovec *rd;
> uint8_t *map;
> @@ -206,13 +207,20 @@ eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> framenum = pkt_q->framenum;
> ppd = (struct tpacket2_hdr *) pkt_q->rd[framenum].iov_base;
> for (i = 0; i < nb_pkts; i++) {
> + mbuf = *bufs++;
> +
> + /* drop oversized packets */
> + if (rte_pktmbuf_data_len(mbuf) > pkt_q->frame_data_size) {
> + rte_pktmbuf_free(mbuf);
> + continue;
> + }
> +
> /* point at the next incoming frame */
> if ((ppd->tp_status != TP_STATUS_AVAILABLE) &&
> (poll(&pfd, 1, -1) < 0))
> - continue;
> + break;
>
> /* copy the tx frame data */
> - mbuf = bufs[num_tx];
> pbuf = (uint8_t *) ppd + TPACKET2_HDRLEN -
> sizeof(struct sockaddr_ll);
> memcpy(pbuf, rte_pktmbuf_mtod(mbuf, void*), rte_pktmbuf_data_len(mbuf));
> @@ -231,13 +239,13 @@ eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>
> /* kick-off transmits */
> if (sendto(pkt_q->sockfd, NULL, 0, MSG_DONTWAIT, NULL, 0) == -1)
> - return 0; /* error sending -- no packets transmitted */
> + num_tx = 0; /* error sending -- no packets transmitted */
>
> pkt_q->framenum = framenum;
> pkt_q->tx_pkts += num_tx;
> - pkt_q->err_pkts += nb_pkts - num_tx;
> + pkt_q->err_pkts += i - num_tx;
> pkt_q->tx_bytes += num_tx_bytes;
> - return num_tx;
> + return i;
> }
>
> static int
> @@ -634,6 +642,8 @@ rte_pmd_init_internals(const char *name,
>
> tx_queue = &((*internals)->tx_queue[q]);
> tx_queue->framecount = req->tp_frame_nr;
> + tx_queue->frame_data_size = req->tp_frame_size;
> + tx_queue->frame_data_size -= TPACKET2_HDRLEN - sizeof(struct sockaddr_ll);
>
> tx_queue->map = rx_queue->map + req->tp_block_size * req->tp_block_nr;
>
> --
> 2.10.2
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Michal Miroslaw @ 2016-12-13 16:14 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E736D@irsmsx105.ger.corp.intel.com>
On Tue, Dec 13, 2016 at 03:13:42PM +0000, Ananyev, Konstantin wrote:
[...]
> > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > > > > >
> > > > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > > > ---
> > > > > > lib/librte_acl/rte_acl.c | 3 +--
> > > > > > lib/librte_acl/rte_acl.h | 2 --
> > > > > > lib/librte_table/rte_table_acl.c | 2 +-
> > > > > > 3 files changed, 2 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > > > > > index 8b7e92c..d1f40be 100644
> > > > > > --- a/lib/librte_acl/rte_acl.c
> > > > > > +++ b/lib/librte_acl/rte_acl.c
> > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > > > > > rd->category_mask) == 0 ||
> > > > > > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > > > > > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > > > > > return -EINVAL;
> > > > > > return 0;
> > > > > > }
> > > > >
> > > > > I am not sure, how it supposed to work properly?
> > > > > Zero value is reserved and ifnicates that no match were found for that input.
> > > >
> > > > This is actually in use by us. In our use we don't need to differentiate
> > > > matching a rule with zero verdict vs not matching a rule at all. I also
> > > > have a patch that changes the value returned in non-matching case, but
> > > > it's in "dirty hack" state, as of yet.
> > >
> > > With that chane rte_acl_classify() might produce invalid results.
> > > Even if you don't need it (I still don't understand how) , it doesn't mean other people
> > > don't need it either and it is ok to change it.
> > >
> > > >
> > > > The ACL code does not treat zero userdata specially, so this is only
> > > > a policy choice and as such would be better to be made by the user.
> > >
> > > I believe it does.
> > > userdata==0 is a reserved value.
> > > When rte_acl_clasify() returns 0 for that particular input, it means 'no matches were found'.
> >
> > Dear Konstantin,
> >
> > Can you describe how the ACL code treats zero specially? I could not find
> > anything, really. The only thing I found is that iff I use zero userdata
> > in a rule I won't be able to differentiate a case where it matched from
> > a case where no rule matched.
>
> Yes, that's what I am talking about.
>
> > If I all my rules have non-zero userdata,
> > then this patch changes nothing.
>
> Ok, then why do you remove a code that does checking for invalid userdata==0?
> That supposed to prevent user to setup invalid value by mistake.
>
> But if I have a table where 0 means drop
> > (default-drop policy) then being able to use zero userdata in DROP rules
> > makes the ACLs just that more useful.
>
> Ok, and what prevents you from do +1 to your policy values before
> you insert it into the ACL table and -1 after you retrieved it via rte_acl_classify()?
The check is enforcing an assumption that all users want to distinguish
the cases whether any rule matched and whether no rules matched. Not all
users do, hence the assumption is invalid and this patch removes it.
Yes, people can work around it by loosing 1 of 2^32 useful values and
convoluting their code.
You seem to argue that 0 is somehow an invalid value, but I can't find
anything in the ACL that would require it to be so. Could you point me
to the code in DPDK where this actually matters?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH v2 08/13] PMD/af_packet: guard against buffer overruns in RX path
From: John W. Linville @ 2016-12-13 16:05 UTC (permalink / raw)
To: Michał Mirosław; +Cc: dev, test-report
In-Reply-To: <a85f2954ecc8ee1fef7d6e88756c6120b55560ed.1481592081.git.mirq-linux@rere.qmqm.pl>
On Tue, Dec 13, 2016 at 02:28:34AM +0100, Michał Mirosław wrote:
>
> Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
Acked-by: John W. Linville <linville@tuxdriver.com>
> ---
> drivers/net/af_packet/rte_eth_af_packet.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index ff45068..5599e02 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -370,18 +370,19 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
> {
> struct pmd_internals *internals = dev->data->dev_private;
> struct pkt_rx_queue *pkt_q = &internals->rx_queue[rx_queue_id];
> - uint16_t buf_size;
> + unsigned int buf_size, data_size;
>
> pkt_q->mb_pool = mb_pool;
>
> /* Now get the space available for data in the mbuf */
> - buf_size = (uint16_t)(rte_pktmbuf_data_room_size(pkt_q->mb_pool) -
> - RTE_PKTMBUF_HEADROOM);
> + buf_size = rte_pktmbuf_data_room_size(pkt_q->mb_pool) - RTE_PKTMBUF_HEADROOM;
> + data_size = internals->req.tp_frame_size;
> + data_size -= TPACKET2_HDRLEN - sizeof(struct sockaddr_ll);
>
> - if (ETH_FRAME_LEN > buf_size) {
> + if (data_size > buf_size) {
> RTE_LOG(ERR, PMD,
> "%s: %d bytes will not fit in mbuf (%d bytes)\n",
> - dev->data->name, ETH_FRAME_LEN, buf_size);
> + dev->data->name, data_size, buf_size);
> return -ENOMEM;
> }
>
> --
> 2.10.2
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH] SDK: Add scripts to initialize DPDK runtime
From: Christian Ehrhardt @ 2016-12-13 15:19 UTC (permalink / raw)
To: Stefan Bader; +Cc: Luca Boccassi, bruce.richardson@intel.com, dev@dpdk.org
In-Reply-To: <22e6b726-79f5-5c2b-2cc6-b11fba6384c7@canonical.com>
Stefans mail got rejected by moderator (??!), reposting FYI as I'm
subscribed and should autopass the moderation.
On Tue, Dec 13, 2016 at 3:45 PM, Stefan Bader <stefan.bader@canonical.com>
wrote:
> On 13.12.2016 08:00, Christian Ehrhardt wrote:
> >
> > On Mon, Dec 12, 2016 at 10:58 PM, Luca Boccassi <lboccass@brocade.com
> > <mailto:lboccass@brocade.com>> wrote:
> >
> > If the 2 authors (CC'ed Stefan, the second one) agree and give
> > permission it could be relicensed to BSD.
> >
> > Stefan, Christian, is that OK for you?
> >
> >
> > To re-license it for this purpose is ok for me, I'll ask Stefan later
> today who
> > was starting on it before me (and added the License initially).
> > Thanks for pushing that forward Luca!
>
> Ok with me, too.
>
> -Stefan
--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Ananyev, Konstantin @ 2016-12-13 15:13 UTC (permalink / raw)
To: Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <20161213145308.lqqnm6ivryjfxih7@rere.qmqm.pl>
> -----Original Message-----
> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> Sent: Tuesday, December 13, 2016 2:53 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
>
> On Tue, Dec 13, 2016 at 02:14:19PM +0000, Ananyev, Konstantin wrote:
> > > -----Original Message-----
> > > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> > > Sent: Tuesday, December 13, 2016 1:55 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > >
> > > On Tue, Dec 13, 2016 at 10:36:16AM +0000, Ananyev, Konstantin wrote:
> > > > Hi Michal,
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > > > To: dev@dpdk.org
> > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > > > >
> > > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > > ---
> > > > > lib/librte_acl/rte_acl.c | 3 +--
> > > > > lib/librte_acl/rte_acl.h | 2 --
> > > > > lib/librte_table/rte_table_acl.c | 2 +-
> > > > > 3 files changed, 2 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > > > > index 8b7e92c..d1f40be 100644
> > > > > --- a/lib/librte_acl/rte_acl.c
> > > > > +++ b/lib/librte_acl/rte_acl.c
> > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > > > > rd->category_mask) == 0 ||
> > > > > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > > > > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > > > > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > > > > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > > > > return -EINVAL;
> > > > > return 0;
> > > > > }
> > > >
> > > > I am not sure, how it supposed to work properly?
> > > > Zero value is reserved and ifnicates that no match were found for that input.
> > >
> > > This is actually in use by us. In our use we don't need to differentiate
> > > matching a rule with zero verdict vs not matching a rule at all. I also
> > > have a patch that changes the value returned in non-matching case, but
> > > it's in "dirty hack" state, as of yet.
> >
> > With that chane rte_acl_classify() might produce invalid results.
> > Even if you don't need it (I still don't understand how) , it doesn't mean other people
> > don't need it either and it is ok to change it.
> >
> > >
> > > The ACL code does not treat zero userdata specially, so this is only
> > > a policy choice and as such would be better to be made by the user.
> >
> > I believe it does.
> > userdata==0 is a reserved value.
> > When rte_acl_clasify() returns 0 for that particular input, it means 'no matches were found'.
>
> Dear Konstantin,
>
> Can you describe how the ACL code treats zero specially? I could not find
> anything, really. The only thing I found is that iff I use zero userdata
> in a rule I won't be able to differentiate a case where it matched from
> a case where no rule matched.
Yes, that's what I am talking about.
> If I all my rules have non-zero userdata,
> then this patch changes nothing.
Ok, then why do you remove a code that does checking for invalid userdata==0?
That supposed to prevent user to setup invalid value by mistake.
But if I have a table where 0 means drop
> (default-drop policy) then being able to use zero userdata in DROP rules
> makes the ACLs just that more useful.
Ok, and what prevents you from do +1 to your policy values before
you insert it into the ACL table and -1 after you retrieved it via rte_acl_classify()?
Konstantin
^ permalink raw reply
* Re: [PATCH v2 12/12] drivers: update PMDs to use rte_driver probe and remove
From: Ferruh Yigit @ 2016-12-13 15:07 UTC (permalink / raw)
To: Andrew Rybchenko, Shreyansh Jain, dev, david.marchand
Cc: thomas.monjalon, jianbo.liu
In-Reply-To: <44213d68-1e87-c464-549a-274e389b9c0f@solarflare.com>
On 12/13/2016 1:52 PM, Andrew Rybchenko wrote:
> On 12/13/2016 04:37 PM, Shreyansh Jain wrote:
>> These callbacks now act as first layer of PCI interfaces from the Bus.
>> Bus probe would enter the PMDs through the rte_driver->probe/remove
>> callbacks, falling to rte_xxx_driver->probe/remove (Currently, all the
>> drivers are rte_pci_driver).
>
> I think similar changes in drivers/net/sfc/sfc_ethdev.c (already in
> dpdk-next-net) are required as well.
Yes, that change is required, but it is a little tricky because this
patchset targets main tree where sfc is not merged yet, so this patch
can't include required patches.
I think it is possible to wait for this patch to be merged into main
tree, and when next-net rebased on top of it, sfc can be patched
individually.
So, yes there is a work to do there, but I think it can be postponed a
little.
>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>> drivers/net/bnx2x/bnx2x_ethdev.c | 8 ++++++++
>> drivers/net/bnxt/bnxt_ethdev.c | 4 ++++
>> drivers/net/cxgbe/cxgbe_ethdev.c | 4 ++++
>> drivers/net/e1000/em_ethdev.c | 4 ++++
>> drivers/net/e1000/igb_ethdev.c | 8 ++++++++
>> drivers/net/ena/ena_ethdev.c | 4 ++++
>> drivers/net/enic/enic_ethdev.c | 4 ++++
>> drivers/net/fm10k/fm10k_ethdev.c | 4 ++++
>> drivers/net/i40e/i40e_ethdev.c | 4 ++++
>> drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
>> drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++++++++
>> drivers/net/mlx4/mlx4.c | 4 +++-
>> drivers/net/mlx5/mlx5.c | 1 +
>> drivers/net/nfp/nfp_net.c | 4 ++++
>> drivers/net/qede/qede_ethdev.c | 8 ++++++++
>> drivers/net/szedata2/rte_eth_szedata2.c | 4 ++++
>> drivers/net/thunderx/nicvf_ethdev.c | 4 ++++
>> drivers/net/virtio/virtio_ethdev.c | 2 ++
>> drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 ++++
>> 19 files changed, 86 insertions(+), 1 deletion(-)
<...>
^ permalink raw reply
* Re: [PATCH 06/13] null: fake PMD capabilities
From: Michal Miroslaw @ 2016-12-13 14:57 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E70A6@irsmsx105.ger.corp.intel.com>
On Tue, Dec 13, 2016 at 02:37:34PM +0000, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> > Sent: Tuesday, December 13, 2016 2:27 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> >
> > On Tue, Dec 13, 2016 at 10:48:32AM +0000, Ananyev, Konstantin wrote:
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> > > >
> > > > From: Paweł Małachowski <pawel.malachowski@atendesoftware.pl>
> > > >
> > > > Thanks to that change we can use Null PMD for testing purposes.
> > > >
> > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > ---
> > > > drivers/net/null/rte_eth_null.c | 13 ++++++++++++-
> > > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> > > > index 836d982..f32ba2a 100644
> > > > --- a/drivers/net/null/rte_eth_null.c
> > > > +++ b/drivers/net/null/rte_eth_null.c
> > > > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> > > > return 0;
> > > > }
> > > >
> > > > +static void
> > > > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > > > +
> > > >
> > > > static void
> > > > eth_dev_info(struct rte_eth_dev *dev,
> > > > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > > > dev_info->pci_dev = NULL;
> > > > dev_info->reta_size = internals->reta_size;
> > > > dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
> > > > + /* We hereby declare we can RX-offload VLAN-s out of thin air and update checksums and VLANs before sinking packets in
> > > > /dev/null */
> > > > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > > > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT | DEV_TX_OFFLOAD_IPV4_CKSUM;
> > >
> > > Hmm, how could it be supported if all that null PMD does on TX - just free the packets?
> > > Same question for RX.
> >
> > You could imagine, that before dropping the packet you insert the tag
> > and calculate the checksum. The observed effect will be the same.
> >
> > On RX this always indicates lack of VLAN tag. So whether the offload
> > is enabled or not it doesn't matter.
>
> Of course user can imagine whatever he likes, but what the point to advertise these capabilities,
> when the PMD clearly doesn't have them?
> Again, why these particular ones?
Hmm. I guess we can expand the set. Those were the ones we actually use
(depend on) for testing our code.
This allows to use null PMD for testing in place of real network interface
with those offloads. This patch is to keep users from having to place if's
around their code just to support test scenarios with null PMD.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Michal Miroslaw @ 2016-12-13 14:53 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E7008@irsmsx105.ger.corp.intel.com>
On Tue, Dec 13, 2016 at 02:14:19PM +0000, Ananyev, Konstantin wrote:
> > -----Original Message-----
> > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> > Sent: Tuesday, December 13, 2016 1:55 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> >
> > On Tue, Dec 13, 2016 at 10:36:16AM +0000, Ananyev, Konstantin wrote:
> > > Hi Michal,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > > To: dev@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > > >
> > > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > > ---
> > > > lib/librte_acl/rte_acl.c | 3 +--
> > > > lib/librte_acl/rte_acl.h | 2 --
> > > > lib/librte_table/rte_table_acl.c | 2 +-
> > > > 3 files changed, 2 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > > > index 8b7e92c..d1f40be 100644
> > > > --- a/lib/librte_acl/rte_acl.c
> > > > +++ b/lib/librte_acl/rte_acl.c
> > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > > > rd->category_mask) == 0 ||
> > > > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > > > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > > > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > > > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > > > return -EINVAL;
> > > > return 0;
> > > > }
> > >
> > > I am not sure, how it supposed to work properly?
> > > Zero value is reserved and ifnicates that no match were found for that input.
> >
> > This is actually in use by us. In our use we don't need to differentiate
> > matching a rule with zero verdict vs not matching a rule at all. I also
> > have a patch that changes the value returned in non-matching case, but
> > it's in "dirty hack" state, as of yet.
>
> With that chane rte_acl_classify() might produce invalid results.
> Even if you don't need it (I still don't understand how) , it doesn't mean other people
> don't need it either and it is ok to change it.
>
> >
> > The ACL code does not treat zero userdata specially, so this is only
> > a policy choice and as such would be better to be made by the user.
>
> I believe it does.
> userdata==0 is a reserved value.
> When rte_acl_clasify() returns 0 for that particular input, it means 'no matches were found'.
Dear Konstantin,
Can you describe how the ACL code treats zero specially? I could not find
anything, really. The only thing I found is that iff I use zero userdata
in a rule I won't be able to differentiate a case where it matched from
a case where no rule matched. If I all my rules have non-zero userdata,
then this patch changes nothing. But if I have a table where 0 means drop
(default-drop policy) then being able to use zero userdata in DROP rules
makes the ACLs just that more useful.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH 06/13] null: fake PMD capabilities
From: Ananyev, Konstantin @ 2016-12-13 14:37 UTC (permalink / raw)
To: Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <20161213142654.me3p5gl7muji3sei@rere.qmqm.pl>
> -----Original Message-----
> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> Sent: Tuesday, December 13, 2016 2:27 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
>
> On Tue, Dec 13, 2016 at 10:48:32AM +0000, Ananyev, Konstantin wrote:
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> > >
> > > From: Paweł Małachowski <pawel.malachowski@atendesoftware.pl>
> > >
> > > Thanks to that change we can use Null PMD for testing purposes.
> > >
> > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > ---
> > > drivers/net/null/rte_eth_null.c | 13 ++++++++++++-
> > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> > > index 836d982..f32ba2a 100644
> > > --- a/drivers/net/null/rte_eth_null.c
> > > +++ b/drivers/net/null/rte_eth_null.c
> > > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> > > return 0;
> > > }
> > >
> > > +static void
> > > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > > +
> > >
> > > static void
> > > eth_dev_info(struct rte_eth_dev *dev,
> > > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > > dev_info->pci_dev = NULL;
> > > dev_info->reta_size = internals->reta_size;
> > > dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
> > > + /* We hereby declare we can RX-offload VLAN-s out of thin air and update checksums and VLANs before sinking packets in
> > > /dev/null */
> > > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT | DEV_TX_OFFLOAD_IPV4_CKSUM;
> >
> > Hmm, how could it be supported if all that null PMD does on TX - just free the packets?
> > Same question for RX.
>
> You could imagine, that before dropping the packet you insert the tag
> and calculate the checksum. The observed effect will be the same.
>
> On RX this always indicates lack of VLAN tag. So whether the offload
> is enabled or not it doesn't matter.
Of course user can imagine whatever he likes, but what the point to advertise these capabilities,
when the PMD clearly doesn't have them?
Again, why these particular ones?
Konstantin
>
> Best Regards,
> Michał Mirosław
^ permalink raw reply
* Re: [PATCH v2 17/32] net/i40e: set VF broadcast mode from PF
From: Iremonger, Bernard @ 2016-12-13 14:35 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, dev@dpdk.org
In-Reply-To: <10f85acc-17d4-4ac3-29d2-6b27430b2240@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 7, 2016 2:33 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 17/32] net/i40e: set VF broadcast mode
> from PF
>
> On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> > Support enabling/disabling VF broadcast mode from PF.
> > User can call the API on PF to enable/disable a specific VF's
> > broadcast mode.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>
> <...>
>
> > +int rte_pmd_i40e_set_vf_broadcast(uint8_t port, uint16_t vf_id,
> > +uint8_t on) {
> > + struct rte_eth_dev *dev;
> > + struct rte_eth_dev_info dev_info;
> > + struct i40e_pf *pf;
> > + struct i40e_pf_vf *vf;
> > + struct i40e_hw *hw;
> > + int ret;
> > +
> > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > + dev = &rte_eth_devices[port];
> > + rte_eth_dev_info_get(port, &dev_info);
> > +
> > + if (vf_id >= dev_info.max_vfs)
> > + return -EINVAL;
> > +
> > + if (on > 1)
> > + return -EINVAL;
> > +
> > + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > + hw = I40E_PF_TO_HW(pf);
> > +
> > + if (vf_id > pf->vf_num)
>
> if (vf_id > pf->vf_num - 1 || !pf->vfs)
if (vf_id >= pf->vf_num || !pf->vfs)
might be better.
>
> > + return -EINVAL;
> > +
>
> <...>
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH 06/13] null: fake PMD capabilities
From: Michal Miroslaw @ 2016-12-13 14:26 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E6E4C@irsmsx105.ger.corp.intel.com>
On Tue, Dec 13, 2016 at 10:48:32AM +0000, Ananyev, Konstantin wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > Sent: Tuesday, December 13, 2016 1:08 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 06/13] null: fake PMD capabilities
> >
> > From: Paweł Małachowski <pawel.malachowski@atendesoftware.pl>
> >
> > Thanks to that change we can use Null PMD for testing purposes.
> >
> > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > ---
> > drivers/net/null/rte_eth_null.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> > index 836d982..f32ba2a 100644
> > --- a/drivers/net/null/rte_eth_null.c
> > +++ b/drivers/net/null/rte_eth_null.c
> > @@ -284,6 +284,9 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
> > return 0;
> > }
> >
> > +static void
> > +eth_dev_void_ok(struct rte_eth_dev *dev __rte_unused) { return; }
> > +
> >
> > static void
> > eth_dev_info(struct rte_eth_dev *dev,
> > @@ -304,6 +307,9 @@ eth_dev_info(struct rte_eth_dev *dev,
> > dev_info->pci_dev = NULL;
> > dev_info->reta_size = internals->reta_size;
> > dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
> > + /* We hereby declare we can RX-offload VLAN-s out of thin air and update checksums and VLANs before sinking packets in
> > /dev/null */
> > + dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
> > + dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT | DEV_TX_OFFLOAD_IPV4_CKSUM;
>
> Hmm, how could it be supported if all that null PMD does on TX - just free the packets?
> Same question for RX.
You could imagine, that before dropping the packet you insert the tag
and calculate the checksum. The observed effect will be the same.
On RX this always indicates lack of VLAN tag. So whether the offload
is enabled or not it doesn't matter.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Ananyev, Konstantin @ 2016-12-13 14:14 UTC (permalink / raw)
To: Michal Miroslaw; +Cc: dev@dpdk.org
In-Reply-To: <20161213135443.ovmlunbh67dr4tew@rere.qmqm.pl>
> -----Original Message-----
> From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl]
> Sent: Tuesday, December 13, 2016 1:55 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
>
> On Tue, Dec 13, 2016 at 10:36:16AM +0000, Ananyev, Konstantin wrote:
> > Hi Michal,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > > Sent: Tuesday, December 13, 2016 1:08 AM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> > >
> > > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > > ---
> > > lib/librte_acl/rte_acl.c | 3 +--
> > > lib/librte_acl/rte_acl.h | 2 --
> > > lib/librte_table/rte_table_acl.c | 2 +-
> > > 3 files changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > > index 8b7e92c..d1f40be 100644
> > > --- a/lib/librte_acl/rte_acl.c
> > > +++ b/lib/librte_acl/rte_acl.c
> > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > > rd->category_mask) == 0 ||
> > > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > > return -EINVAL;
> > > return 0;
> > > }
> >
> > I am not sure, how it supposed to work properly?
> > Zero value is reserved and ifnicates that no match were found for that input.
>
> This is actually in use by us. In our use we don't need to differentiate
> matching a rule with zero verdict vs not matching a rule at all. I also
> have a patch that changes the value returned in non-matching case, but
> it's in "dirty hack" state, as of yet.
With that chane rte_acl_classify() might produce invalid results.
Even if you don't need it (I still don't understand how) , it doesn't mean other people
don't need it either and it is ok to change it.
>
> The ACL code does not treat zero userdata specially, so this is only
> a policy choice and as such would be better to be made by the user.
I believe it does.
userdata==0 is a reserved value.
When rte_acl_clasify() returns 0 for that particular input, it means 'no matches were found'.
Konstantin
^ permalink raw reply
* Re: [PATCH 04/13] acl: allow zero verdict
From: Michal Miroslaw @ 2016-12-13 13:54 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev@dpdk.org
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0E6E0B@irsmsx105.ger.corp.intel.com>
On Tue, Dec 13, 2016 at 10:36:16AM +0000, Ananyev, Konstantin wrote:
> Hi Michal,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miroslaw
> > Sent: Tuesday, December 13, 2016 1:08 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict
> >
> > Signed-off-by: Michał Mirosław <michal.miroslaw@atendesoftware.pl>
> > ---
> > lib/librte_acl/rte_acl.c | 3 +--
> > lib/librte_acl/rte_acl.h | 2 --
> > lib/librte_table/rte_table_acl.c | 2 +-
> > 3 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> > index 8b7e92c..d1f40be 100644
> > --- a/lib/librte_acl/rte_acl.c
> > +++ b/lib/librte_acl/rte_acl.c
> > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data *rd)
> > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
> > rd->category_mask) == 0 ||
> > rd->priority > RTE_ACL_MAX_PRIORITY ||
> > - rd->priority < RTE_ACL_MIN_PRIORITY ||
> > - rd->userdata == RTE_ACL_INVALID_USERDATA)
> > + rd->priority < RTE_ACL_MIN_PRIORITY)
> > return -EINVAL;
> > return 0;
> > }
>
> I am not sure, how it supposed to work properly?
> Zero value is reserved and ifnicates that no match were found for that input.
This is actually in use by us. In our use we don't need to differentiate
matching a rule with zero verdict vs not matching a rule at all. I also
have a patch that changes the value returned in non-matching case, but
it's in "dirty hack" state, as of yet.
The ACL code does not treat zero userdata specially, so this is only
a policy choice and as such would be better to be made by the user.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH v12] net/tap: new TUN/TAP device PMD
From: Ferruh Yigit @ 2016-12-13 13:54 UTC (permalink / raw)
To: Keith Wiles, dev; +Cc: pmatilai, yuanhan.liu, john.mcnamara
In-Reply-To: <20161212143838.37269-1-keith.wiles@intel.com>
On 12/12/2016 2:38 PM, Keith Wiles wrote:
> The PMD allows for DPDK and the host to communicate using a raw
> device interface on the host and in the DPDK application. The device
> created is a Tap device with a L2 packet header.
>
> v12- Fixup minor changes for driver_name and version number
> v11- Add the tap.rst to the nic/index.rst file
> v10- Change the string name used to allow for multiple devices.
> v9 - Fix up the docs to use correct syntax
> v8 - Fix issue with tap_tx_queue_setup() not return zero on success.
> v7 - Reword the comment in common_base and fix the data->name issue
> v6 - fixed the checkpatch issues
> v5 - merge in changes from list review see related emails
> fixed many minor edits
> v4 - merge with latest driver changes
> v3 - fix includes by removing ifdef for other type besides Linux
> Fix the copyright notice in the Makefile
> v2 - merge all of the patches into one patch
> Fix a typo on naming the tap device
> Update the maintainers list
>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Aws Ismail <aismail@ciena.com>
Tested-by: Vasily Philipov <vasilyf@mellanox.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply
* Re: [PATCH v2 12/12] drivers: update PMDs to use rte_driver probe and remove
From: Andrew Rybchenko @ 2016-12-13 13:52 UTC (permalink / raw)
To: Shreyansh Jain, dev, david.marchand
Cc: thomas.monjalon, ferruh.yigit, jianbo.liu
In-Reply-To: <1481636232-2300-13-git-send-email-shreyansh.jain@nxp.com>
On 12/13/2016 04:37 PM, Shreyansh Jain wrote:
> These callbacks now act as first layer of PCI interfaces from the Bus.
> Bus probe would enter the PMDs through the rte_driver->probe/remove
> callbacks, falling to rte_xxx_driver->probe/remove (Currently, all the
> drivers are rte_pci_driver).
I think similar changes in drivers/net/sfc/sfc_ethdev.c (already in
dpdk-next-net) are required as well.
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
> drivers/net/bnx2x/bnx2x_ethdev.c | 8 ++++++++
> drivers/net/bnxt/bnxt_ethdev.c | 4 ++++
> drivers/net/cxgbe/cxgbe_ethdev.c | 4 ++++
> drivers/net/e1000/em_ethdev.c | 4 ++++
> drivers/net/e1000/igb_ethdev.c | 8 ++++++++
> drivers/net/ena/ena_ethdev.c | 4 ++++
> drivers/net/enic/enic_ethdev.c | 4 ++++
> drivers/net/fm10k/fm10k_ethdev.c | 4 ++++
> drivers/net/i40e/i40e_ethdev.c | 4 ++++
> drivers/net/i40e/i40e_ethdev_vf.c | 4 ++++
> drivers/net/ixgbe/ixgbe_ethdev.c | 8 ++++++++
> drivers/net/mlx4/mlx4.c | 4 +++-
> drivers/net/mlx5/mlx5.c | 1 +
> drivers/net/nfp/nfp_net.c | 4 ++++
> drivers/net/qede/qede_ethdev.c | 8 ++++++++
> drivers/net/szedata2/rte_eth_szedata2.c | 4 ++++
> drivers/net/thunderx/nicvf_ethdev.c | 4 ++++
> drivers/net/virtio/virtio_ethdev.c | 2 ++
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 4 ++++
> 19 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
> index 0eae433..9f3b3f2 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -618,6 +618,10 @@ eth_bnx2xvf_dev_init(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_bnx2x_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_bnx2x_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> .probe = rte_eth_dev_pci_probe,
> @@ -632,6 +636,10 @@ static struct eth_driver rte_bnx2x_pmd = {
> */
> static struct eth_driver rte_bnx2xvf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_bnx2xvf_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 035fe07..c8671c8 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -1160,6 +1160,10 @@ bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
>
> static struct eth_driver bnxt_rte_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = bnxt_pci_id_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> index b7f28eb..67714fa 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -1039,6 +1039,10 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_cxgbe_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = cxgb4_pci_tbl,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index aee3d34..7be5da3 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -391,6 +391,10 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_em_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_em_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 2fddf0c..70dd24c 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1078,6 +1078,10 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_igb_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_igb_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> @@ -1094,6 +1098,10 @@ static struct eth_driver rte_igb_pmd = {
> */
> static struct eth_driver rte_igbvf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_igbvf_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index ab9a178..54fc8de 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1705,6 +1705,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>
> static struct eth_driver rte_ena_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_ena_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index 2b154ec..c2783db 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -634,6 +634,10 @@ static int eth_enicpmd_dev_init(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_enic_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_enic_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 923690c..d1a2efa 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -3061,6 +3061,10 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
>
> static struct eth_driver rte_pmd_fm10k = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_fm10k_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 67778ba..9c5d50f 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -670,6 +670,10 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
>
> static struct eth_driver rte_i40e_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_i40e_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index aa306d6..10bf6ab 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1527,6 +1527,10 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
> */
> static struct eth_driver rte_i40evf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_i40evf_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index edc9b22..80ee232 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1564,6 +1564,10 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_ixgbe_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_ixgbe_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> @@ -1580,6 +1584,10 @@ static struct eth_driver rte_ixgbe_pmd = {
> */
> static struct eth_driver rte_ixgbevf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_ixgbevf_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index da61a85..e3dcd41 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -5907,7 +5907,9 @@ static const struct rte_pci_id mlx4_pci_id_map[] = {
> static struct eth_driver mlx4_driver = {
> .pci_drv = {
> .driver = {
> - .name = MLX4_DRIVER_NAME
> + .name = MLX4_DRIVER_NAME,
> + .probe = rte_eal_pci_probe,
> + },
> },
> .id_table = mlx4_pci_id_map,
> .probe = mlx4_pci_probe,
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 90cc35e..76dda13 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -731,6 +731,7 @@ static struct eth_driver mlx5_driver = {
> .pci_drv = {
> .driver = {
> .name = MLX5_DRIVER_NAME
> + .probe = rte_eal_pci_probe,
> },
> .id_table = mlx5_pci_id_map,
> .probe = mlx5_pci_probe,
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index de80b46..125ba86 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -2469,6 +2469,10 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
>
> static struct eth_driver rte_nfp_net_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_nfp_net_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> RTE_PCI_DRV_DETACHABLE,
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index d106dd0..31f6733 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -1642,6 +1642,10 @@ static struct rte_pci_id pci_id_qede_map[] = {
>
> static struct eth_driver rte_qedevf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_qedevf_map,
> .drv_flags =
> RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> @@ -1655,6 +1659,10 @@ static struct eth_driver rte_qedevf_pmd = {
>
> static struct eth_driver rte_qede_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_qede_map,
> .drv_flags =
> RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
> index f3cd52d..a649e60 100644
> --- a/drivers/net/szedata2/rte_eth_szedata2.c
> +++ b/drivers/net/szedata2/rte_eth_szedata2.c
> @@ -1572,6 +1572,10 @@ static const struct rte_pci_id rte_szedata2_pci_id_table[] = {
>
> static struct eth_driver szedata2_eth_driver = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = rte_szedata2_pci_id_table,
> .probe = rte_eth_dev_pci_probe,
> .remove = rte_eth_dev_pci_remove,
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
> index 466e49c..72ac748 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -2110,6 +2110,10 @@ static const struct rte_pci_id pci_id_nicvf_map[] = {
>
> static struct eth_driver rte_nicvf_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_nicvf_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> .probe = rte_eth_dev_pci_probe,
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 079fd6c..4d5d1bb 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1377,6 +1377,8 @@ static struct eth_driver rte_virtio_pmd = {
> .pci_drv = {
> .driver = {
> .name = "net_virtio",
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> },
> .id_table = pci_id_virtio_map,
> .drv_flags = RTE_PCI_DRV_DETACHABLE,
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 8bb13e5..57f66cb 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -335,6 +335,10 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
>
> static struct eth_driver rte_vmxnet3_pmd = {
> .pci_drv = {
> + .driver = {
> + .probe = rte_eal_pci_probe,
> + .remove = rte_eal_pci_remove,
> + },
> .id_table = pci_id_vmxnet3_map,
> .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
> .probe = rte_eth_dev_pci_probe,
^ permalink raw reply
* fm10k pmd limitations
From: Shaham Fridenberg @ 2016-12-13 13:49 UTC (permalink / raw)
To: dev@dpdk.org
Hey guys,
I'm using dpdk 16.4 and fm10k card. According to the code, there's no support for disabling vlan stripping and VLAN QinQ in pmd fm10k.
Does anybody know why? If there's any way to work-around it, or when is a behavior change expected?
I need my VF to receive the packets with the VLAN headers.
Even if it's possible to change this configurations through the linux kernel fm10k driver?
Thanks!
^ permalink raw reply
* Re: [PATCH v4 0/7] net/ixgbe: move set VF functions.
From: Iremonger, Bernard @ 2016-12-13 13:46 UTC (permalink / raw)
To: Yigit, Ferruh, thomas.monjalon@6wind.com, dev@dpdk.org
In-Reply-To: <763dea95-72d3-c182-8da2-e283ae0a4000@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, December 13, 2016 1:37 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 0/7] net/ixgbe: move set VF functions.
>
> On 12/13/2016 11:40 AM, Bernard Iremonger wrote:
> > This patchset implements the following deprecation notice:
> > [PATCH v1] doc: announce API and ABI change for librte_ether
> >
> > Changes in V4:
> > Fixed compile issues when ixgbe PMD is not present.
> > Removed duplicate testpmd commands.
> > Added cleanup patch for testpmd.
> > Updated release note.
> >
> > Changes in V3:
> > Updated LIBABIVER in Makefile in librte_ether patch.
> > Updated rte_ethdev.h and ret_ether_version.map in librte_ether patch.
> > Squashed deprecation notice patch into librte_ether patch.
> > Added release_note patch.
> >
> > Changes in V2:
> > Update testpmd set vf commands help messages.
> > Updated ethtool to use the ixgbe public API's.
> > Removed the ixgbe_set_pool_* and ixgbe_set_vf_rate_limit functions.
> > Removed the rte_eth_dev_set_vf_* API's Removed the deprecation
> notice.
> >
> > Changes in V1:
> > The following functions from eth_dev_ops have been moved to the ixgbe
> > PMD and renamed:
> >
> > ixgbe_set_pool_rx_mode
> > ixgbe_set_pool_rx
> > ixgbe_set_pool_tx
> > ixgbe_set_pool_vlan_filter
> > ixgbe_set_vf_rate_limit
> >
> > Renamed the functions to the following:
> >
> > rte_pmd_ixgbe_set_vf_rxmode
> > rte_pmd_ixgbe_set_vf_rx
> > rte_pmd_ixgbe_set_vf_tx
> > rte_pmd_ixgbe_set_vf_vlan_filter
> > rte_pmd_ixgbe_set_vf_rate_limit
> >
> > Testpmd has been modified to use the following functions:
> > rte_pmd_ixgbe_set_vf_rxmode
> > rte_pmd_ixgbe_set_vf_rate_limit
> >
> > New testpmd commands have been added to test the following functions:
> > rte_pmd_ixgbe_set_vf_rx
> > rte_pmd_ixgbe_set_vf_tx
> > rte_pmd_ixgbe_set_vf_vlan_filter
> >
> > The testpmd user guide has been updated for the new commands.
> >
> > Bernard Iremonger (7):
> > net/ixgbe: move set VF functions from the ethdev
> > app/testpmd: use ixgbe public functions
> > app/testpmd: cleanup parameter checking
> > examples/ethtool: use ixgbe public function
> > net/ixgbe: remove static set VF functions
> > librte_ether: remove the set VF API's
> > doc: update release notes
> >
>
> Series applied to dpdk-next-net/master, thanks.
>
> Last patch squashed.
Thanks.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v2 25/32] app/testpmd: handle i40e in VF VLAN filter command
From: Ferruh Yigit @ 2016-12-13 13:40 UTC (permalink / raw)
To: Wenzhuo Lu, dev; +Cc: Bernard Iremonger
In-Reply-To: <1481081535-37448-26-git-send-email-wenzhuo.lu@intel.com>
Hi Wenzhuo,
On 12/7/2016 3:32 AM, Wenzhuo Lu wrote:
> modify set_vf_rx_vlan function to handle the i40e PMD.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
Latest applied patches [1] conflict with some testpmd patches of this
patchset.
Can you please rebase this patchset on top of the latest next-net?
[1]
http://dpdk.org/dev/patchwork/patch/17896 - 17902
Thanks,
ferruh
> app/test-pmd/config.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 36c47ab..0368dc6 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -92,6 +92,9 @@
> #include <rte_ethdev.h>
> #include <rte_string_fns.h>
> #include <rte_cycles.h>
> +#ifdef RTE_LIBRTE_I40E_PMD
> +#include <rte_pmd_i40e.h>
> +#endif
>
> #include "testpmd.h"
>
> @@ -2349,12 +2352,24 @@ struct igb_ring_desc_16_bytes {
> set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
> {
> int diag;
> + struct rte_eth_dev_info dev_info;
>
> if (port_id_is_invalid(port_id, ENABLED_WARN))
> return;
> if (vlan_id_is_invalid(vlan_id))
> return;
> - diag = rte_eth_dev_set_vf_vlan_filter(port_id, vlan_id, vf_mask, on);
> +
> + rte_eth_dev_info_get(port_id, &dev_info);
> +
> +#ifdef RTE_LIBRTE_I40E_PMD
> + if (strstr(dev_info.driver_name, "i40e") != NULL)
> + diag = rte_pmd_i40e_set_vf_vlan_filter(port_id, vlan_id,
> + vf_mask, on);
> + else
> +#endif
> + diag = rte_eth_dev_set_vf_vlan_filter(port_id, vlan_id,
> + vf_mask, on);
> +
> if (diag == 0)
> return;
> printf("rte_eth_dev_set_vf_vlan_filter for port_id=%d failed "
>
^ 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