Hi Dave,
This patchset contains misc cleanups and improvements
to the mlx4 Core and Eth drivers.
In patches 1 and 2 I reduce and reorder the branches in the RX csum flow.
In patch 3 I align the FMR unmapping flow with the device spec, to allow
a remapping afterwards.
Patch 4 by Moni changes the default QoS settings so that a pause
frame stops all traffic regardless of its prio.
Series generated against net-next commit:
836df24a7062 net: hns3: hns3_get_channels() can be static
Thanks,
Tariq.
Moni Shoua (1):
net/mlx4_en: Change default QoS settings
Tariq Toukan (3):
net/mlx4_en: RX csum, remove redundant branches and checks
net/mlx4_en: RX csum, reorder branches
net/mlx4_core: Cleanup FMR unmapping flow
drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c | 5 +++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 7 ++++
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 56 +++++++++++++-------------
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 1 +
drivers/net/ethernet/mellanox/mlx4/mr.c | 40 +++++++++---------
5 files changed, 63 insertions(+), 46 deletions(-)
--
1.8.3.1
Use early goto commands, and save else branches.
This uses less indentations and brackets, making the code
more readable.
Signed-off-by: Tariq Toukan <redacted>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 46 ++++++++++++++----------------
1 file changed, 21 insertions(+), 25 deletions(-)
Do not check IPv6 bit in cqe status if CONFIG_IPV6 is not enabled.
Function check_csum() is reached only with IPv4 or IPv6 set (if enabled),
if IPv6 is not set (or is not enabled) it is redundant to test the
IPv4 bit.
Signed-off-by: Tariq Toukan <redacted>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
@@ -617,6 +617,10 @@ static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff *skb,return0;}#endif++/* We reach this function only after checking that any of+*the(IPv4|IPv6)bitsaresetincqe->status.+*/staticintcheck_csum(structmlx4_cqe*cqe,structsk_buff*skb,void*va,netdev_features_tdev_features){
Remove redundant and not essential operations in fmr unmap/free.
According to device spec, in FMR unmap it is sufficient to set
ownership bit to SW. This allows remapping afterwards.
Fixes: 8ad11fb6b073 ("IB/mlx4: Implement FMRs")
Signed-off-by: Tariq Toukan <redacted>
Signed-off-by: Moshe Shemesh <redacted>
---
drivers/net/ethernet/mellanox/mlx4/mr.c | 40 +++++++++++++++++----------------
1 file changed, 21 insertions(+), 19 deletions(-)
@@ -1103,30 +1103,16 @@ int mlx4_fmr_enable(struct mlx4_dev *dev, struct mlx4_fmr *fmr)voidmlx4_fmr_unmap(structmlx4_dev*dev,structmlx4_fmr*fmr,u32*lkey,u32*rkey){-structmlx4_cmd_mailbox*mailbox;-interr;-if(!fmr->maps)return;-fmr->maps=0;+/* To unmap: it is sufficient to take back ownership from HW */+*(u8*)fmr->mpt=MLX4_MPT_STATUS_SW;-mailbox=mlx4_alloc_cmd_mailbox(dev);-if(IS_ERR(mailbox)){-err=PTR_ERR(mailbox);-pr_warn("mlx4_ib: mlx4_alloc_cmd_mailbox failed (%d)\n",err);-return;-}+/* Make sure MPT status is visible */+wmb();-err=mlx4_HW2SW_MPT(dev,NULL,-key_to_hw_index(fmr->mr.key)&-(dev->caps.num_mpts-1));-mlx4_free_cmd_mailbox(dev,mailbox);-if(err){-pr_warn("mlx4_ib: mlx4_HW2SW_MPT failed (%d)\n",err);-return;-}-fmr->mr.enabled=MLX4_MPT_EN_SW;+fmr->maps=0;}EXPORT_SYMBOL_GPL(mlx4_fmr_unmap);
@@ -1136,6 +1122,22 @@ int mlx4_fmr_free(struct mlx4_dev *dev, struct mlx4_fmr *fmr)if(fmr->maps)return-EBUSY;+if(fmr->mr.enabled==MLX4_MPT_EN_HW){+/* In case of FMR was enabled and unmapped+*makesuretogiveownershipofMPTbacktoHW+*soHW2SW_MPTcommandwillsuccess.+*/+*(u8*)fmr->mpt=MLX4_MPT_STATUS_SW;+/* Make sure MPT status is visible before changing MPT fields */+wmb();+fmr->mpt->length=0;+fmr->mpt->start=0;+/* Make sure MPT data is visible after changing MPT status */+wmb();+*(u8*)fmr->mpt=MLX4_MPT_STATUS_HW;+/* make sure MPT status is visible */+wmb();+}ret=mlx4_mr_free(dev,&fmr->mr);if(ret)
This patchset contains misc cleanups and improvements
to the mlx4 Core and Eth drivers.
In patches 1 and 2 I reduce and reorder the branches in the RX csum flow.
In patch 3 I align the FMR unmapping flow with the device spec, to allow
a remapping afterwards.
Patch 4 by Moni changes the default QoS settings so that a pause
frame stops all traffic regardless of its prio.
Series generated against net-next commit:
836df24a7062 net: hns3: hns3_get_channels() can be static
Series applied, thanks Tariq.
I can't say that that ipv6 ifdef you added in patch #1 is the nicest.
It's one thing to ifdef control entire blocks of code, or individual
values.
It's another thing to partially include pieces of code structure
such as closing parenthesis, arithmetic operators, and curly braces.
Two of those were included in the ifdef section.
For example, if we have:
if (x & (IPV4_THING | IPV6_THING)) {
I don't want to see:
if (x & (IPV4_THING |
#ifdef IPV6_TEST
IPV6_THING)) {
#else
0)) {
#endif
Those closing parenthesis and the openning curly brace are there
regardless of the CPP test, and duplicating them in both arms of
the CPP test only makes the code more confusing to read.
I can say I would prefer:
if (x & (IPV4_THING |
#ifdef IPV6_TEST
IPV6_THING
#else
0
#endif
)) {
which is better. However, best would be:
#ifdef IPV6_TEST
#define THING_MASK IPV4_THING | IPV6_THING
#else
#define THING_MASK IPV4_THING
#endif
if (x & THING_MASK) {
is the best.
This patchset contains misc cleanups and improvements
to the mlx4 Core and Eth drivers.
In patches 1 and 2 I reduce and reorder the branches in the RX csum flow.
In patch 3 I align the FMR unmapping flow with the device spec, to allow
a remapping afterwards.
Patch 4 by Moni changes the default QoS settings so that a pause
frame stops all traffic regardless of its prio.
Series generated against net-next commit:
836df24a7062 net: hns3: hns3_get_channels() can be static
Series applied, thanks Tariq.
I can't say that that ipv6 ifdef you added in patch #1 is the nicest.
It's one thing to ifdef control entire blocks of code, or individual
values.
It's another thing to partially include pieces of code structure
such as closing parenthesis, arithmetic operators, and curly braces.
Two of those were included in the ifdef section.
For example, if we have:
if (x & (IPV4_THING | IPV6_THING)) {
I don't want to see:
if (x & (IPV4_THING |
#ifdef IPV6_TEST
IPV6_THING)) {
#else
0)) {
#endif
Those closing parenthesis and the openning curly brace are there
regardless of the CPP test, and duplicating them in both arms of
the CPP test only makes the code more confusing to read.
I can say I would prefer:
if (x & (IPV4_THING |
#ifdef IPV6_TEST
IPV6_THING
#else
0
#endif
)) {
which is better. However, best would be:
#ifdef IPV6_TEST
#define THING_MASK IPV4_THING | IPV6_THING
#else
#define THING_MASK IPV4_THING
#endif
if (x & THING_MASK) {
is the best.
Hi Dave,
Thanks for accepting the series.
Your suggestions look better indeed, I will prepare a followup patch and submit soon.
Regards,
Tariq