ort
> > app/testpmd: add NVGRE encap/decap support
> >
> > app/test-pmd/cmdline.c | 169 +
> > app/test-pmd/cmdline_flow.c | 248
> > app/test-pmd/testpmd.c | 28 +++
> > app/test-pmd/testpmd.h | 28 +++
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 25 ++
> > 5 files changed, 498 insertions(+)
--
Nélio Laranjeiro
6WIND
On Mon, Jun 18, 2018 at 03:40:53PM +0100, Ferruh Yigit wrote:
> On 6/18/2018 10:38 AM, Nélio Laranjeiro wrote:
> > On Mon, Jun 18, 2018 at 10:05:03AM +0100, Ferruh Yigit wrote:
> >> On 6/18/2018 9:52 AM, Nelio Laranjeiro wrote:
> >>> This series adds an easy and main
== 0)
> > + vxlan_encap_conf.select_vlan = 1;
> > + if (strcmp(res->ip_version, "ipv4") == 0)
> > + vxlan_encap_conf.select_ipv4 = 1;
> > + else if (strcmp(res->ip_version, "ipv6") == 0)
> > + vxlan_encap_conf.select_ipv4 = 0;
> > + else
> > + return;
> > + memcpy(vxlan_encap_conf.vni, &vni, 3);
>
> I don't think this line is correct when running on big endian system.
Yes, this is wrong, it will be fixed in v4.
Thanks,
--
Nélio Laranjeiro
6WIND
index 0 / end
> >
> > set nvgre ipv6 4 ::1 :: 11:11:11:11:11:11 22:22:22:22:22:22 flow
> > create 0
> > ingress pattern end actions nvgre_encap / queue index 0 / end
> >
>
> It might be useful to add the above sample testpmd command lines to
> section 4.12 of the doc/guides/testpmd_app_ug/testpmd_funcs.rst file
>[...]
Agreed, I'll add it in the v4.
Thanks,
--
Nélio Laranjeiro
6WIND
g to enable it).
I'll make the modification for the v4,
Thanks,
--
Nélio Laranjeiro
6WIND
On Mon, Jun 18, 2018 at 05:06:41PM +, Yongseok Koh wrote:
>
> > On Jun 7, 2018, at 12:39 AM, Nélio Laranjeiro
> > wrote:
> >
> > On Wed, Jun 06, 2018 at 11:39:27AM -0700, Yongseok Koh wrote:
> >> On Wed, Jun 06, 2018 at 08:55:01AM +0200, Nélio Laranjei
.ibv_attr)
> rte_free(parser.queue[i].ibv_attr);
> }
> - rte_errno = ret; /* Restore rte_errno. */
> + if (ret)
> + rte_errno = ret; /* Restore rte_errno. */
> return -rte_errno;
> }
>
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
otherwise.
letting the function documentation saying rte_errno is only modified in
case of error whereas it is not is a bug or in the documentation or in
the code, but as a function must respect its documentation, it would
have raised a bug in the code itself.
Regards,
--
Nélio Laranjeiro
6WIND
t; The above changes (if they are the only one) can be address by me when I
> apply the patch.
> Will wait few days for the maintainers comments.
>
> Acked-by: Shahaf Shuler
We should answer the same as the other patches, there is already a
re-work remodeling the whole engine, this patch remains relevant for
stables branches, but no more for mainline.
Regards,
--
Nélio Laranjeiro
6WIND
> flow create 0 ingress pattern eth / ipv4 src is 10.2.3.4 / end
actions vxlan_encap / queue index 0 / end
testpmd> flow create 0 ingress pattern eth / ipv4 src is 20.2.3.4 / end
actions vxlan_encap / queue index 0 / end
will encapsulate the packets having as IPv4 source IP 10.2.3.4 and
20.2.3.4 with the same VXLAN tunnel headers.
Regards,
--
Nélio Laranjeiro
6WIND
On Fri, Jun 22, 2018 at 09:51:15AM +0100, Mohammad Abdul Awal wrote:
>
>
> On 22/06/2018 09:31, Nélio Laranjeiro wrote:
> > On Fri, Jun 22, 2018 at 08:42:10AM +0100, Mohammad Abdul Awal wrote:
> > > Hi Nelio,
> > >
> > >
> > > On 21/06/2018
FLOAD_IP_TNL_TSO |
> DEV_TX_OFFLOAD_UDP_TNL_TSO |
>DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) &
> --
> 2.12.0
>
Is not it a fix?
Regards,
--
Nélio Laranjeiro
6WIND
On Mon, Jun 25, 2018 at 11:23:22AM +, Shahaf Shuler wrote:
> Monday, June 25, 2018 9:41 AM , Nélio Laranjeiro:
> > Subject: Re: [PATCH] net/mlx5: separate generic tunnel TSO from the
> > standard one
> >
> > On Sun, Jun 24, 2018 at 09:22:26AM +0300, Shahaf Shul
On Fri, Jun 22, 2018 at 11:19:14AM +0100, Mohammad Abdul Awal wrote:
> On 22/06/2018 10:08, Nélio Laranjeiro wrote:
> > On Fri, Jun 22, 2018 at 09:51:15AM +0100, Mohammad Abdul Awal wrote:
> > >
> > > On 22/06/2018 09:31, Nélio Laranjeiro wrote:
> > > > O
0.1 11:11:11:11:11:11
> > + 22:22:22:22:22:22 testpmd> flow create 0 ingress pattern end actions
> > + nvgre_encap / queue index 0 / end
> > +
> > + testpmd> set nvgre-with-vlan 4 127.0.0.1 128.0.0.1 34
> > + 11:11:11:11:11:11 22:22:22:22:22:22 testpmd> flow
Sorry, I've messed up with my local branches. I will send a v7 which
only fixes the compilation issues on redhat.
Thanks,
--
Nélio Laranjeiro
6WIND
ret = mlx5_get_ifname(dev, &ifname);
> if (ret)
> return ret;
> ret = if_nametoindex(ifname);
> - if (ret == -1) {
> + if (ret == 0) {
> rte_errno = errno;
> return -rte_errno;
> }
> --
> 2.11.0
Thanks,
--
Nélio Laranjeiro
6WIND
ns(+), 1 deletion(-)
> create mode 100644 drivers/net/mlx5/mlx5_nl_flow.c
>
> --
> 2.11.0
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
ram dev
> > + * Pointer to Ethernet device.
> > + * @param hrxq
> > + * Pointer to Hash Rx queue to release.
> > + */
> > +void
> > +mlx5_hrxq_drop_release(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq)
>[...]
>
> hrxq is a redundant argument. Can be referenced by priv->drop.hrxq.
>[...]
Ditto.
Thanks,
--
Nélio Laranjeiro
6WIND
On Tue, Jul 03, 2018 at 10:05:05AM -0700, Yongseok Koh wrote:
> On Tue, Jul 03, 2018 at 09:17:56AM +0200, Nélio Laranjeiro wrote:
> > On Mon, Jul 02, 2018 at 06:07:03PM -0700, Yongseok Koh wrote:
> > > On Wed, Jun 27, 2018 at 05:07:34PM +0200, Nelio Laranjeiro wrote:
> [...]
&
+ * Apply the flow.
> > + *
> > + * @param dev
> > + * Pointer to Ethernet device structure.
> > + * @param flow
> > + * Pointer to flow structure.
> > + * @param error
> > + * Pointer to error structure.
> > + *
> > + * @return
> > +
RTE_FLOW_ERROR_TYPE_ITEM_SPEC,
> > + item->spec,
> > + "VLAN TPID matching is not"
> > + " supported");
>
> Not sure 100% but I don't think ether_type means TPID but the inner packet
> type
> coming after the VLAN ID. E.g.
>
> /dmac/smac/0x8100/TCI/0x0800/ipv4...
> ^
> |
I remember to have faced such issue several months ago, from what I
understood it is configurable but there is no way in Verbs to make it.
Currently (after testing) it matches the above logic, I will remove this
check.
> > + if (!(flow->layers & l2m)) {
> > + if (size <= flow_size)
> > + mlx5_flow_spec_verbs_add(flow, ð, size);
> > + } else {
> > + if (flow->verbs.attr)
> > + mlx5_flow_item_vlan_update(flow->verbs.attr, ð);
> > + size = 0; /**< Only an update is done in eth specification. */
>
> Any specific reason to use doxygen style comment only here?
No reason, I'll remove it.
Thanks,
--
Nélio Laranjeiro
6WIND
> void
> > mlx5_flow_stop(struct rte_eth_dev *dev, struct mlx5_flows *list)
> > {
> > + struct priv *priv = dev->data->dev_private;
> > struct rte_flow *flow;
> > + unsigned int i;
> > + unsigned int idx;
> >
> > TAILQ_FOREACH_REVERSE(flow, list, mlx5_flows, next)
> > mlx5_flow_fate_remove(dev, flow);
> > + for (idx = 0, i = 0; idx != priv->rxqs_n; ++i) {
> > + if (!(*priv->rxqs)[idx])
> > + continue;
> > + (*priv->rxqs)[idx]->mark = 0;
> > + ++idx;
> > + }
>
> Same question here but looks like this part is being moved to
> mlx5_flow_rxqs_clear() in the future.
Addressing both question here, for the flow_stop() and flow_destroy()
the process is different, for the stop, the flow remains with the mark
bit set but all queues must me cleared, there is no comparison to make.
As you can see, it don't even get a flow, it directly unset the mask bit
in the Rx queues.
For the destroy the issue is different, several flows may be using the
same Rx queues, if one of them will remains and has a mark, then the
associated queues must keep their mark bit set.
As the process is different, it would end in two distinct functions and
each one used by a single function.
For the mlx5_flow_rxq_mark(), the situation is different, the same
process is make when a flow is created and the flow are started.
> > }
> >
> > /**
> > @@ -1386,6 +1594,7 @@ mlx5_flow_start(struct rte_eth_dev *dev, struct
> > mlx5_flows *list)
> > ret = mlx5_flow_fate_apply(dev, flow, &error);
> > if (ret < 0)
> > goto error;
> > + mlx5_flow_rxq_mark(dev, flow);
> > }
> > return 0;
> > error:
> > --
> > 2.18.0
Thanks,
--
Nélio Laranjeiro
6WIND
"set vxlan ipv4 4 4 4 127.0.0.1 128.0.0.1
> > 11:11:11:11:11:11 22:22:22:22:22:22", it does not look much user
> > friendly to me. A user may easily lose track of sequence of 9 param
> > items. It would be much user friendly if the options would be like below
> > and has auto-completion.
> >
> > set vxlan ip_ver vni udp_src
> > udp-dst ip_src ip_dst
> > eth_src eth_dst
>
> Hi Nelio, Adrien,
>
> I tend to agree with Awal here, this is to forget/confuse and key-value pairs
> makes it easier to use.
>
> Meanwhile this is an usability improvement and I prefer not to block this
> patch
> for this.
>
> What is your comment on this, how should we proceed?
>
> Thanks,
> ferruh
Hi,
I also agree with this proposal, I'll prepare a v8 with those fix
tokens.
> > This way an user may never feel confused. Can maintainers comment on
> > this point please?
> >
> > Regards,
> > Awal.
Thanks
--
Nélio Laranjeiro
6WIND
On Thu, Jul 05, 2018 at 04:07:28PM +0100, Mohammad Abdul Awal wrote:
>Some nits.
>
>Auto-completion suggestion for values should be wrapped between '<' and
>'>', not '(' and ')'. See all the cases.
>[...]
Right, I'll send a v9 to fix this.
Thanks,
--
Nélio Laranjeiro
6WIND
same
> > process is make when a flow is created and the flow are started.
>
> I knew the differences but I just wanted to ask if having a separate function
> can be a viable option, e.g.,
>
> mlx5_flow_rxq_mark_set()
> mlx5_flow_rxq_mark_clear()
> mlx5_flow_rxq_mark_trim()
Certainly, the point is those functions have a short life as few patches
letter they will be removed.
I suppose you prefer to have them and I don't think it will take too
much time to add such function, it will make part of the next revision
;).
Thanks,
--
Nélio Laranjeiro
6WIND
difier &= ~(MLX5_FLOW_MOD_FLAG | MLX5_FLOW_MOD_MARK);
>
> Can't understand this well. Is this for the case where the flow is expanded?
> If
> so, why don't you reset flow->modifier in the for loop of mlx5_flow_merge()?
Yes it is, I'll move it.
>[...]
> > + assert(ret > 0);
> > + buf = rte_calloc(__func__, 1, ret, 0);
> > + if (!buf) {
> > + rte_flow_error_set(error, ENOMEM,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL,
> > + "not enough memory to expand the RSS flow");
> > + goto error;
> > + }
>
> I'm pretty sure you've already fixed this bug. Validation can't return ENOMEM.
You know me well ;)
Thanks,
--
Nélio Laranjeiro
6WIND
On Fri, Jul 06, 2018 at 05:35:22PM +, Yongseok Koh wrote:
>
> > On Jul 6, 2018, at 8:59 AM, Nélio Laranjeiro
> > wrote:
> >
> > Hi Yongseok,
> >
> > I am only addressing your questions concerns here, almost all other
> > points I also agree w
> What if protocol in IP header does have wrong value other than 47
> (IPPROTO_GRE)?
> Shouldn't we have a validation check for it in mlx5_flow_item_gre()?
>[...]
Already added, the same issue occurs also with UDP/TCP. If the user
uses some protocol it must match the following layer, otherwise its
request won't be respected which is a bug.
Thanks,
--
Nélio Laranjeiro
6WIND
om Mellanox PMD team but from anyone proposing the drop. The
chances he breaks anything if the code is shared among several items is
high. It is better to have a single function per item/action according
to the API directly.
Thanks,
[1] https://datatracker.ietf.org/doc/draft-quinn-vxlan-gpe/
--
Nélio Laranjeiro
6WIND
, error);
> > break;
>
> #ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT
>
> > + case RTE_FLOW_ITEM_TYPE_MPLS:
> > + ret = mlx5_flow_item_mpls(items, flow, remain, error);
> > + break;
>
> #endif /* !HAVE_IBV_DEVICE_MPLS_SUPPORT */
>
> How about this?
>[...]
It adds another couple of #ifdef #endif and the final output won't help
much the user, having an error "MPLS is not updated by Verbs, please
update" will help more than "item not supported".
Regards,
--
Nélio Laranjeiro
6WIND
he .c file should also have the experimental tag.
>
> > rte_flow_expand_rss(struct rte_flow_expand_rss *buf, size_t size,
> > const struct rte_flow_item *pattern, uint64_t types,
> > const struct rte_flow_expand_node graph[],
> > --
Will fix both point in a v2.
Thanks,
--
Nélio Laranjeiro
6WIND
/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1328,23 +1328,20 @@ struct ethtool_link_settings {
> ifname);
>
> file = fopen(phys_port_name, "rb");
> - if (file == NULL)
> - goto error;
> - port_name_set = fscanf(file, "%d%c", &data.port_name, &c) == 2 &&
> - c == '\n';
> - fclose(file);
> + if (file) {
> + port_name_set = fscanf(file, "%d%c", &data.port_name, &c) ==
> 2 &&
> + c == '\n';
> + fclose(file);
> + }
> file = fopen(phys_switch_id, "rb");
> - if (file == NULL)
> - goto error;
> - port_switch_id_set =
> - fscanf(file, "%" SCNx64 "%c", &data.switch_id, &c) == 2 &&
> - c == '\n';
> - fclose(file);
> + if (file) {
> + port_switch_id_set =
> + fscanf(file, "%" SCNx64 "%c", &data.switch_id, &c) ==
> 2 &&
> + c == '\n';
> + fclose(file);
> + }
> data.master = port_switch_id_set && !port_name_set;
> data.representor = port_switch_id_set && port_name_set;
Regards,
--
Nélio Laranjeiro
6WIND
the
application, the number of queues says how are in used, it does not mean
they are contiguous in the rxqs arrays and this due to the DPDK API
which configure the number of queues with rte_eth_dev_configure()
whereas queues are instantiated with rte_eth_rx_queue_setup() which
takes an position in the array as parameter.
Indeed this code is wrong, idx should always increase whereas i should
only increase if the (*priv->rxqs)[idx] is non null.
--
Nélio Laranjeiro
6WIND
u want another glue library. It won't be for this release in this
case, I don't have time to write such glue.
> > else
> > _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -
> > libverbs -lmlx5 -lmnl
> > endif
> > --
> > 2.18.0
>
[1] https://mails.dpdk.org/archives/dev/2018-March/092876.html
--
Nélio Laranjeiro
6WIND
On Tue, Jul 24, 2018 at 09:47:19PM +, Yongseok Koh wrote:
>
> > On Jul 23, 2018, at 11:57 PM, Nélio Laranjeiro
> > wrote:
> >
> > On Mon, Jul 23, 2018 at 11:27:44AM -0700, Yongseok Koh wrote:
> >> If one of (*priv->rxqs)[] is null, the for loop can
tion: 'Extra compiler flags')
> > +option('extra_ldflags', type: 'string', description: 'Extra linker flags')
>
> This should not be needed. Meson should pick up CFLAGS and LDFLAGS from the
> environment without having to add options for them.
>
> https://mesonbuild.com/howtox.html#set-extra-compiler-and-linker-flags-from-the-outside-when-eg-building-distro-packages
>
> /Bruce
Indeed this works with the CLFAGS/LDFLAGS way, but to find correctly the
library dependencies, it also needs to have the LD_LIBRARY_PATH set with
the correct path.
This patch will be discarded in the next version.
Thanks,
--
Nélio Laranjeiro
6WIND
'-Wl,-h,@0@'.format(LIB_GLUE),
> > + '-lmlx4',
> > + '-libverbs',
>
> While this works, the recommended approach is to save the return value from
> cc.find_library() above, and pass that as a dependency directly, rather
> than as a linker flag.
I tried it, but:
drivers/net/mlx5/meson.build:216:8: ERROR: Link_args arguments must be
strings.
find_library returns a compiler object, I did not found anyway to use
directly the output of the find_library which works in places.
Thanks,
--
Nélio Laranjeiro
6WIND
gt; > > PMD it will
> > > +# use some variables here to configure it.
> > > +pmd_dlopen = get_option('enable_driver_mlx4_glue')
> > > +build = get_option('enable_driver_mlx4') or pmd_dlopen
> >
> > As stated above, I believe this should be based upon whether you find
> > the
> > "mnl", "mlx4" and "ibverbs" libraries. If we start adding back in
> > static
> > options for every driver, then we'll be back to having a mass of
> > config
> > options like we had before.
>
> BTW, slightly related to that: ibverbs doesn't ship pkg-config files at
> the moment which makes the detection slightly more awkward that it
> could be, so I've sent a PR upstream to add that:
>
> https://github.com/linux-rdma/rdma-core/pull/373
>
> Hope this can be useful!
Thanks Luca, I was also searching for it, you save me some time, I hope
this can be backported down to RDMA-Core's stable version v15 of
RDMA-Core it would fully help.
--
Nélio Laranjeiro
6WIND
On Wed, Aug 29, 2018 at 11:01:15AM +0100, Bruce Richardson wrote:
> On Wed, Aug 29, 2018 at 11:34:10AM +0200, Nélio Laranjeiro wrote:
> > Hi Bruce,
> >
> > Thanks for your comments I have address almost all of them in the v3 by
> > doing what you suggest, I still hav
cript.
> >
> > Signed-off-by: Nelio Laranjeiro
> >
>
> Couple of minor comments inline below. Otherwise:
>
> Acked-by: Bruce Richardson
>[...]
Hi Bruce,
I'll address those comments in a v4, thanks a lot for your help and
review.
--
Nélio Laranjeiro
6WIND
ers/net/failsafe/failsafe_flow.c | 31 +-
> drivers/net/failsafe/failsafe_private.h| 5 +-
> lib/librte_ethdev/rte_ethdev_version.map | 1 +
> lib/librte_ethdev/rte_flow.c | 666 ++++++--
> lib/librte_ethdev/rte_flow.h | 231 +++-
> 15 files changed, 886 insertions(+), 539 deletions(-)
>
> --
> 2.11.0
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
On Tue, Apr 17, 2018 at 04:53:15AM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Adrien Mazarguil
> > Sent: Tuesday, April 17, 2018 12:03 AM
> > To: Xueming(Steven) Li
> > Cc: Nélio Laranjeiro ; Shahaf Shuler
> > ; d
e.
>
> This patch aligns with testpmd by making a deep copy instead.
>
> Fixes: 18da437b5f63 ("ethdev: add flow rule copy function")
> Cc: sta...@dpdk.org
> Cc: Gaetan Rivet
>
> Signed-off-by: Adrien Mazarguil
> Cc: Thomas Monjalon
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
f->rss_hf),
> - rss_conf->rss_key, rss->num, rss->queue);
> + (priv, fields, rss_conf->rss_key, rss->num,
> + rss->queue);
> if (!flow->rss) {
> msg = "either invalid parameters or not enough"
> " resources for additional multi-queue"
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>"flow rule handle allocation failure");
> + }
> /* Most fields will be updated by second pass. */
> *flow = (struct rte_flow){
> .ibv_attr = temp.ibv_attr,
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
;hugepage_sz);
>
> DRV_LOG(DEBUG,
> "port %u mempool %p using start=%p end=%p size=%zu for memory"
> --
> 2.11.0
>
--
Nélio Laranjeiro
6WIND
_flow.c| 25 ---
> > lib/librte_ether/rte_flow.h| 8 ++-
> > 8 files changed, 135 insertions(+), 117 deletions(-)
>
> There are almost as much insertions as deletions.
> So it's probably not a bad move.
>
> Acked-by: Thomas Monjalon
For mlx5: Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
iguration of the firmware. If the firmware is not correctly
configured the PMD must refuse such rule.
Thanks,
--
Nélio Laranjeiro
6WIND
tunnel type identifier.
As discussed in the previous thread, you cannot set all tunnel bits in
the mbuf, it will break existing applications.
This is an non announce API breakage.
Thanks,
--
Nélio Laranjeiro
6WIND
oto is 47 / gre / end
> actions rss queues 1 2 end level 1 / end
>
> GRE tunnel flow outer RSS:
> flow create 0 ingress pattern eth / ipv4 proto is 47 / gre / end
> actions rss queues 1 2 end level 0 / end
>
> Signed-off-by: Xueming Li
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
flow->frxq[i].hash_fields |
> + (flow->tunnel &&
> + flow->rss_conf.level > 1 ? (uint32_t)IBV_RX_HASH_INNER : 0),
> + flow->rss_conf.queue_num,
> + flow->frxq[i].ibv_attr->num_of_specs,
> + flow->frxq[i].ibv_attr->size,
> + flow->frxq[i].ibv_attr->priority,
> + flow->frxq[i].ibv_attr->type,
> + flow->frxq[i].ibv_attr->flags,
> + flow->frxq[i].ibv_attr->comp_mask,
> + buf);
> +#endif
>[...]
Thanks,
--
Nélio Laranjeiro
6WIND
9 insertions(+), 3 deletions(-)
>[...]
--
Nélio Laranjeiro
6WIND
IP_OVER_VXLAN
> + IP_OVER_VXLAN_EN True(1)
> + IP_OVER_VXLAN_PORT 4790
> +
> Prerequisites
> -
>
> --
> 2.13.3
The documentation modification related to the L3 VXLAN should be in the
same patch as the code in mlx5.
Thanks,
--
Nélio Laranjeiro
6WIND
On Wed, Apr 18, 2018 at 02:33:01PM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Wednesday, April 18, 2018 2:51 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
On Wed, Apr 18, 2018 at 02:43:30PM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Wednesday, April 18, 2018 2:49 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
On Thu, Apr 19, 2018 at 06:20:50AM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Wednesday, April 18, 2018 11:09 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
On Thu, Apr 19, 2018 at 10:21:26AM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Thursday, April 19, 2018 2:56 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
On Thu, Apr 19, 2018 at 11:53:05AM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Thursday, April 19, 2018 7:15 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
On Thu, Apr 19, 2018 at 12:49:41PM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Thursday, April 19, 2018 8:19 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
i.e. the
traffic remains untouched.
> > + for (i = MLX5_MAX_UC_MAC_ADDRESSES; i !=
> > MLX5_MAX_MAC_ADDRESSES; ++i)
> > + mlx5_internal_mac_addr_remove(dev, i);
> > + i = MLX5_MAX_UC_MAC_ADDRESSES;
> > + while (nb_mc_addr--) {
>
> Maybe worth checking is_multicast_ether_addr(mc_addr_set) and to skip
> + warn if it is not.
Such verification should be done in the public API i.e. ethdev.
> > + ret = mlx5_internal_mac_addr_add(dev, mc_addr_set++,
> > i++);
> > + if (ret)
> > + return ret;
> > + }
> > + if (!dev->data->promiscuous)
> > + return mlx5_traffic_restart(dev);
> > + return 0;
> > +}
> > --
> > 2.17.0
Regards,
--
Nélio Laranjeiro
6WIND
unce and breaks API/ABI. It cannot be
accepted yet.
I'll suggest to add a new RTE_PTYPE_TUNNEL_UNKNOWN which does not break
the ABI or don't add such bits in the mbuf.
Regards,
--
Nélio Laranjeiro
6WIND
On Mon, Apr 23, 2018 at 07:57:36AM +, Shahaf Shuler wrote:
> Monday, April 23, 2018 10:33 AM, Nélio Laranjeiro:
> [...]
> > > > +/**
> > > > + * DPDK callback to set multicast addresses list.
> > > > + *
> > > > + * @param
rxq_init_n)
> + return rte_flow_error_set(data->error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "IP protocol of GRE must be 47");
> + mlx5_flow_create_copy(parser, &tunnel, size);
> + return 0;
> +}
There is something strange, item is not unused as it is at least used in
the rte_flow_error_set().
In the other series you are pushing, there is no new RTE_FLOW_ITEM_GRE
and in the current code there is also no RTE_FLOW_ITEM_GRE.
I don't see how this code can match the missing item, what am I missing?
> +/**
> * Convert mark/flag action to Verbs specification.
> *
> * @param parser
> --
> 2.13.3
Thanks,
--
Nélio Laranjeiro
6WIND
On Mon, Apr 23, 2018 at 01:32:23PM +, Xueming(Steven) Li wrote:
> Hi Nelio,
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Monday, April 23, 2018 8:56 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> &
f(parser,
> - (const struct rte_eth_rss_conf *)
> - &priv->rss_conf);
This is still needed for QUEUE action, Verbs refuses to create an hash
Rx queue if no RSS key is provided even if the hash field is 0.
This can be fully moved to mlx5_hrxq_new() who can use the default key
when the rss_key is not provided.
Regards,
--
Nélio Laranjeiro
6WIND
On Tue, Apr 24, 2018 at 07:40:24AM +, Xueming(Steven) Li wrote:
>
>
> > -Original Message-
> > From: Nélio Laranjeiro
> > Sent: Monday, April 23, 2018 9:46 PM
> > To: Xueming(Steven) Li
> > Cc: Shahaf Shuler ; dev@dpdk.org
> > Subject:
t; drivers/net/mlx5/mlx5_utils.h | 6 +
> 16 files changed, 896 insertions(+), 223 deletions(-)
>
> --
> 2.13.3
>
I think we have caught almost all issues, if something remains fixes can
be added.
Acked-by: Nelio Laranjeiro
--
Nélio Laranjeiro
6WIND
attr = parser.queue[HASH_RXQ_ETH].ibv_attr;
> - flow_attr = flow->frxq[HASH_RXQ_ETH].ibv_attr;
> + attr = parser.queue[parser.layer].ibv_attr;
> + flow_attr = flow->frxq[parser.layer].ibv_attr;
> /* Compare first the attributes. */
> if (memcmp(attr, flow_attr, sizeof(struct ibv_flow_attr)))
> continue;
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
to the link speed.
> +}
> +
> +/**
> * DPDK callback to get information about the device.
> *
> * @param dev
> @@ -458,6 +508,7 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
> info->hash_key_size = rss_hash_default_key_len;
> info->speed_capa = priv->link_speed_capa;
> info->flow_type_rss_offloads = ~MLX5_RSS_HF_MASK;
> + mlx5_set_default_params(dev, info);
> }
>
> /**
> --
> 2.12.0
Thanks,
--
Nélio Laranjeiro
6WIND
update_unlocked_gset(dev, &dev_link);
> if (ret == 0)
> break;
> /* Handle wait to complete situation. */
> --
> 2.12.0
>
--
Nélio Laranjeiro
6WIND
DPDK callback to get information about the device.
> *
> * @param dev
> @@ -458,6 +497,7 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
> info->hash_key_size = rss_hash_default_key_len;
> info->speed_capa = priv->link_speed_capa;
> info->flow_type_rss_offloads = ~MLX5_RSS_HF_MASK;
> + mlx5_set_default_params(dev, info);
> }
>
> /**
> --
> 2.12.0
>
Thanks,
--
Nélio Laranjeiro
6WIND
if (err)
> > + if (err) {
> > + err = -err;
> > goto error;
> > + }
> > /*
> > * Ethdev pointer is still required as input since
> > * the primary device is not accessible from the
> > --
> > 2.12.0
> >
Regards,
--
Nélio Laranjeiro
6WIND
cket_connect(struct rte_eth_dev *dev)
> }
> ret = *fd;
> close(socket_fd);
> - return 0;
> + return ret;
> error:
> if (socket_fd != -1)
> close(socket_fd);
> --
> 2.12.0
>
--
Nélio Laranjeiro
6WIND
er should be reset.
> + */
> + priv = eth_dev->data->dev_private;
> + priv->dev = eth_dev;
> err = mlx5_uar_init_secondary(eth_dev);
> if (err)
> goto error;
> --
> 2.11.0
>
--
Nélio Laranjeiro
6WIND
goto error;
>
> Do we really need to spec check?
> Meaning if above one passes it is guarantee m is contained in mask.
> And if so, then the spec check will always succeed.
Indeed,
> > }
> > return 0;
> > +error:
> > + rte_errno = ENOTSUP;
> > + return -rte_errno;
> > }
> >
> > /**
> > --
> > 2.17.0
I am making a v2 accordingly.
Thanks,
--
Nélio Laranjeiro
6WIND
MTU is %u", eth_dev->data->port_id,
> priv->mtu);
> /*
> @@ -1031,6 +1043,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
> __rte_unused,
> if (err) {
> DRV_LOG(ERR, "port %u drop queue allocation failed: %s",
> eth_dev->data->port_id, strerror(rte_errno));
> + err = rte_errno;
> goto port_error;
> }
> /* Supported Verbs flow priority number detection. */
> --
> 2.12.0
Unless the small comment above,
Acked-by: Nelio Laranjeiro
Thanks,
--
Nélio Laranjeiro
6WIND
ls as explained in the commit message of [1].
> >
> > Are you sure you encountered this 32b compilation issue for the first time?
On mlx5 32 bits has been disabled as Mellanox OFED does not support
32bits compilation whereas RDMA-Core supports it.
I've just taken a look on Mellanox Website, Mellanox OFED 4.3-3.0.2.1 is
still not available for 32bits. We cannot assume the support exists.
> I do just compilation on mlx drivers.
> And building only mlx4 for 32bits, mlx5 doesn't support 32bits as you point
> out
> below patch. mlx4 32bit compiles fine with me as same config you have used.
>
> Also features documentation [2] verifies this, mlx4 supports 32bits but mlx5
> not.
>
> [2]
> https://dpdk.org/browse/next/dpdk-next-net/tree/doc/guides/nics/features/mlx4.ini?h=v18.02#n32
> https://dpdk.org/browse/next/dpdk-next-net/tree/doc/guides/nics/features/mlx5.ini?h=v18.02#n42
>
> >
> >
> > [1]
> > http://dpdk.org/browse/dpdk/commit/?id=ebbb81eb27daca0a89ee8f228fcf141d9eb6ef1c
> >
> >
> > Thanks,
> > Yongseok
> >
> >
>
--
Nélio Laranjeiro
6WIND
PCI_DEVICE_ID_MELLANOX_CONNECTX5BF)
> + },
> + {
> .vendor_id = 0
> }
> };
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index c4c962b92d..a9c692555e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -50,6 +50,7 @@ enum {
> PCI_DEVICE_ID_MELLANOX_CONNECTX5VF = 0x1018,
> PCI_DEVICE_ID_MELLANOX_CONNECTX5EX = 0x1019,
> PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF = 0x101a,
> + PCI_DEVICE_ID_MELLANOX_CONNECTX5BF = 0xa2d2,
> };
>
> LIST_HEAD(mlx5_dev_list, priv);
> --
> 2.12.0
>
--
Nélio Laranjeiro
6WIND
n &&
> parser->rss_conf.level > 1) {
> rte_flow_error_set(error, ENOTSUP,
Doing such comparison will not work in all cases like
GRE / VOID / MPLS
which is totally valid from rte_flow perspective.
Regards,
--
Nélio Laranjeiro
6WIND
c_mpls mpls = {
> + .type = IBV_FLOW_SPEC_MPLS,
> + .size = size,
> + };
> +
> + parser->inner = IBV_FLOW_SPEC_INNER;
> + if (parser->layer == HASH_RXQ_UDPV4 ||
> + parser->layer == HASH_RXQ_UDPV6) {
> + parser->tunnel =
> + ptype_ext[PTYPE_IDX(RTE_PTYPE_TUNNEL_MPLS_IN_UDP)];
> + parser->out_layer = parser->layer;
> + } else {
> + parser->tunnel =
> + ptype_ext[PTYPE_IDX(RTE_PTYPE_TUNNEL_MPLS_IN_GRE)];
> + /* parser->out_layer stays as in GRE out_layer. */
> + }
> + parser->layer = HASH_RXQ_TUNNEL;
> + /*
> + * For MPLS-in-GRE, RSS level should have been set.
> + * For MPLS-in-UDP, use outer RSS.
> + */
> + if (!parser->rss_conf.level)
> + parser->rss_conf.level = 1;
> + if (spec) {
> + if (!mask)
> + mask = default_mask;
> + /*
> + * The verbs label field includes the entire MPLS header:
> + * bits 0:19 - label value field.
> + * bits 20:22 - traffic class field.
> + * bits 23 - bottom of stack bit.
> + * bits 24:31 - ttl field.
> + */
> + mpls.val.label = *(const uint32_t *)spec;
> + mpls.mask.label = *(const uint32_t *)mask;
> + /* Remove unwanted bits from values. */
> + mpls.val.label &= mpls.mask.label;
> + }
> + mlx5_flow_create_copy(parser, &mpls, size);
> + return 0;
> +#endif
> +}
> +
> +/**
> * Convert mark/flag action to Verbs specification.
> *
> * @param parser
> --
> 1.9.5
>
--
Nélio Laranjeiro
6WIND
t_create(struct rte_eth_dev *dev,
> flow->tunnel = parser.tunnel;
> flow->rss_conf = (struct rte_flow_action_rss){
> .func = RTE_ETH_HASH_FUNCTION_DEFAULT,
> - .level = 0,
> + .level = parser.rss_conf.level,
> .types = parser.rss_conf.types,
> .key_len = parser.rss_conf.key_len,
> .queue_num = parser.rss_conf.queue_num,
> --
> 2.12.0
>
--
Nélio Laranjeiro
6WIND
rte_eth_xstat *stats,
> stats[i].value = (counters[i] - xstats_ctrl->base[i]);
> }
> }
> - return n;
> + return xstats_n;
> }
>
> /**
> --
> 2.7.4
Thanks,
--
Nélio Laranjeiro
6WIND
> > drivers/net/mlx5/mlx5_rxq.c | 221 +++
> > drivers/net/mlx5/mlx5_rxtx.h |6 +
> > 5 files changed, 1388 insertions(+), 2466 deletions(-)
> >
> > --
> > 2.17.0
> >
>
> Regards,
> Keith
Thanks,
[1] https://dpdk.org/dev/patchwork/project/dpdk/list/?submitter=243
--
Nélio Laranjeiro
6WIND
> I have installed MLNX_OFED _LINUX-4.2-1.2.0 on my Ubuntu 14.04 machine but
> still hitting the same error. Am I missing some other package?
Which version of DPDK are you using (it is important to help)?
Regards,
--
Nélio Laranjeiro
6WIND
-
> From: Shahaf Shuler [mailto:shah...@mellanox.com]
> Sent: Thursday, May 31, 2018 10:51 AM
> To: Nitin Katiyar ; Nélio Laranjeiro
>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] Compilation of MLX5 driver
>
> Wednesday, May 30, 2018 7:45 PM, Nitin Katiyar:
>
essage-----
> From: Nélio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> Sent: Thursday, May 31, 2018 1:36 PM
> To: Nitin Katiyar
> Cc: Shahaf Shuler ; dev@dpdk.org
> Subject: Re: [dpdk-dev] Compilation of MLX5 driver
>
> On Thu, May 31, 2018 at 07:01:17AM +, Nitin Kati
id such support, it would be great, otherwise I am afraid
such support cannot be added back without creating a lot of issues for
people wanting it on 32bits processors with Mellanox OFED.
Thanks,
[1]
https://dpdk.org/browse/dpdk/commit/?id=ebbb81eb27daca0a89ee8f228fcf141d9eb6ef1c
--
Nélio Laranjeiro
6WIND
ret = rte_errno; /* Save rte_errno before cleanup. */
> if (flow)
> mlx5_flow_list_destroy(dev, &priv->flows, flow);
> exit:
> --
> 2.11.0
This patch is not enough, the returned value being -rte_errno if no
error is detected by the function it cannot set rte_errno nor return it.
Thanks,
--
Nélio Laranjeiro
6WIND
On Tue, Jun 05, 2018 at 09:36:32PM +, Yongseok Koh wrote:
> > On Jun 4, 2018, at 11:52 PM, Nélio Laranjeiro
> > wrote:
> >
> > On Mon, Jun 04, 2018 at 10:37:31AM -0700, Yongseok Koh wrote:
> >> rte_errno should be saved only if error has occurred because
On Wed, Jun 06, 2018 at 11:39:27AM -0700, Yongseok Koh wrote:
> On Wed, Jun 06, 2018 at 08:55:01AM +0200, Nélio Laranjeiro wrote:
> > On Tue, Jun 05, 2018 at 09:36:32PM +, Yongseok Koh wrote:
> > > > On Jun 4, 2018, at 11:52 PM, Nélio Laranjeiro
> > > > wrote
priv->config.ctrl_flow_prio,
> };
> struct rte_flow_item items[] = {
> {
> @@ -3739,7 +3751,7 @@ unsigned int
> mlx5_get_max_verbs_prio(struct rte_eth_dev *dev)
> {
> struct priv *priv = dev->data->dev_private;
> - unsigned int verb_priorities = MLX5_VERBS_FLOW_PRIO_8;
> + unsigned int verb_priorities = MLX5_VERBS_FLOW_MIN_PRIOS;
> struct {
> struct ibv_flow_attr attr;
> struct ibv_flow_spec_eth eth;
> @@ -3773,8 +3785,7 @@ mlx5_get_max_verbs_prio(struct rte_eth_dev *dev)
> break;
> }
> } while (1);
> - DRV_LOG(DEBUG, "port %u Verbs flow priorities: %d,"
> - " user flow priorities: %d",
> - dev->data->port_id, verb_priorities, MLX5_CTRL_FLOW_PRIORITY);
> + DRV_LOG(DEBUG, "port %u Verbs flow priorities: %d",
> + dev->data->port_id, verb_priorities);
> return verb_priorities;
> }
> --
> 2.13.3
[1] http://dpdk.org/ml/archives/dev/2018-May/103043.html
Regards,
--
Nélio Laranjeiro
6WIND
encapsulation over the inner L2 data definition.
> *
> - * Note: the last field is not used in the definition of a tunnel and can be
> - * ignored.
> + * The tunnel definition is provided through the use of buffer that
> + * holds the encapsulating header.
> + * Provided header must be a valid outer tunnel header.
> + */
> +struct rte_flow_action_tunnel_encap_l3 {
> + enum rte_flow_tunnel_type type; /**< The tunnel type. */
> + void *buf; /**< The header to be used. */
> + uint32_t len; /**< The buf len. */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> *
> - * Valid flow definition for RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP include:
> + * RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP_L3
> *
> - * - ETH / IPV4 / NVGRE / END
> - * - ETH / VLAN / IPV6 / NVGRE / END
> + * Tunnel end-point dencapsulation data definition.
> + * after the decapsulation, the L2 of the resulted packet
> + * is replaced with the supplied buffer.
> *
> + * The tunnel type must match the flow rule spec.
> */
> -struct rte_flow_action_nvgre_encap {
> - /**
> - * Encapsulating vxlan tunnel definition
> - * (terminated by the END pattern item).
> - */
> - struct rte_flow_item *definition;
> +struct rte_flow_action_tunnel_decap_l3 {
> + enum rte_flow_tunnel_type type; /**< The tunnel type. */
> + void *buf; /**< The L2 header to be used.*/
> + uint32_t len; /**< The len of the buf. */
> };
>
> /*
> --
> 1.7.1
>
[1] http://dpdk.org/dev/patchwork/patch/40965/
--
Nélio Laranjeiro
6WIND
gt; > -Original Message-
> > From: Nélio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > Sent: Monday, June 11, 2018 9:53 AM
> > To: Ori Kam
> > Cc: ferruh.yi...@intel.com; declan.dohe...@intel.com; dev@dpdk.org;
> > Adrien Mazarguil
> >
mlx5_set_link_up(eth_dev);
> + mlx5_link_update(eth_dev, 1);
> /* Store device configuration on private structure. */
> priv->config = config;
> continue;
> --
> 2.12.0
--
Nélio Laranjeiro
6WIND
7;t ask them
directly.
> > -Original Message-
> > From: Nélio Laranjeiro [mailto:nelio.laranje...@6wind.com]
> > Sent: Monday, March 26, 2018 7:40 PM
> > To: Xueming(Steven) Li
> > Cc: Adrien Mazarguil ; Shahaf Shuler
> > ; dev@dpdk.org
> > Subject: Re
u configure DPDK (modification you have done the .config file).
Thanks,
--
Nélio Laranjeiro
6WIND
On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote:
> Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro:
> > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> >
> > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote:
> > > Follo
On Thu, Apr 05, 2018 at 05:35:57AM +, Shahaf Shuler wrote:
> Wednesday, April 4, 2018 3:11 PM, Nélio Laranjeiro:
> > Subject: Re: [PATCH] net/mlx5: fix link status initialization
> >
> > On Wed, Apr 04, 2018 at 09:58:33AM +, Shahaf Shuler wrote:
> > > Wed
" context");
> + return -rte_errno;
> + }
> for (n = 0; n < rss->num; ++n) {
> if (rss->queue[n] >= priv->rxqs_n) {
> rte_flow_error_set(error, EINVAL,
> --
> 2.11.0
--
Nélio Laranjeiro
6WIND
101 - 200 of 466 matches
Mail list logo
