* Re: mt7612 suspend/resume issue
From: Lorenzo Bianconi @ 2020-06-18 11:18 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: Lorenzo Bianconi, Felix Fietkau, Ryder Lee, Kalle Valo,
David S. Miller, Jakub Kicinski, Matthias Brugger, linux-wireless,
netdev, linux-mediatek, linux-kernel
In-Reply-To: <20200618090556.pepjdbnba2gqzcbe@butterfly.localdomain>
[-- Attachment #1: Type: text/plain, Size: 5362 bytes --]
> Hello, Lorenzo et al.
Hi Oleksandr,
>
> I'm using MT7612 mini-PCIE cards as both AP in a home server and as a client in
> a laptop. The AP works perfectly (after some fixing from your side; thanks for
> that!), and so does the client modulo it has issues during system resume.
>
[...]
>
> The Wi-Fi becomes unusable from this point. If I `modprobe -r` the "mt76x2e" module
> after this splat, the system hangs completely.
>
> If the system resumes fine, the resume itself takes quite some time (more than
> 10 seconds).
>
> I've found a workaround for this, though. It seems the system behaves fine if I
> do `modprobe -r mt76x2e` before it goes to sleep, and then `modprobe mt76x2e`
> after resume. Also, the resume time improves greatly.
>
> I cannot say if it is some regression or not. I've installed the card
> just recently, and used it with v5.7 kernel series only.
>
> Do you have any idea what could go wrong and how to approach the issue?
I have not reproduced the issue myself yet, but I guess we can try:
1- update to latest Felix's tree [1]
2- could you please try to apply the patch below? (compile-test only)
Regards,
Lorenzo
[1] https://github.com/nbd168/wireless
From 400268a0ee5843cf736308504dfbd2f20a291eaf Mon Sep 17 00:00:00 2001
Message-Id: <400268a0ee5843cf736308504dfbd2f20a291eaf.1592478809.git.lorenzo@kernel.org>
From: Lorenzo Bianconi <lorenzo@kernel.org>
Date: Thu, 18 Jun 2020 13:10:11 +0200
Subject: [PATCH] mt76: mt76x2: fix pci suspend
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../net/wireless/mediatek/mt76/mt76x02_dma.h | 1 +
.../net/wireless/mediatek/mt76/mt76x02_mmio.c | 15 +++++
.../net/wireless/mediatek/mt76/mt76x2/pci.c | 58 +++++++++++++++++++
3 files changed, 74 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h b/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
index 4aff4f8e87b6..6262f2ecded5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_dma.h
@@ -62,5 +62,6 @@ mt76x02_wait_for_wpdma(struct mt76_dev *dev, int timeout)
int mt76x02_dma_init(struct mt76x02_dev *dev);
void mt76x02_dma_disable(struct mt76x02_dev *dev);
void mt76x02_dma_cleanup(struct mt76x02_dev *dev);
+void mt76x02_dma_reset(struct mt76x02_dev *dev);
#endif /* __MT76x02_DMA_H */
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index cbbe986655fe..e2631c964331 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -348,6 +348,21 @@ void mt76x02_dma_disable(struct mt76x02_dev *dev)
}
EXPORT_SYMBOL_GPL(mt76x02_dma_disable);
+void mt76x02_dma_reset(struct mt76x02_dev *dev)
+{
+ int i;
+
+ mt76x02_dma_disable(dev);
+ usleep_range(1000, 2000);
+
+ for (i = 0; i < __MT_TXQ_MAX; i++)
+ mt76_queue_tx_cleanup(dev, i, true);
+ mt76_for_each_q_rx(&dev->mt76, i)
+ mt76_queue_rx_reset(dev, i);
+
+ mt76x02_dma_enable(dev);
+}
+
void mt76x02_mac_start(struct mt76x02_dev *dev)
{
mt76x02_mac_reset_counters(dev);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
index 53ca0cedf026..5543e242fb9b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/pci.c
@@ -103,6 +103,60 @@ mt76pci_remove(struct pci_dev *pdev)
mt76_free_device(mdev);
}
+static int __maybe_unused
+mt76x2e_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+ struct mt76_dev *mdev = pci_get_drvdata(pdev);
+ struct mt76x02_dev *dev = container_of(mdev, struct mt76x02_dev, mt76);
+ int i, err;
+
+ napi_disable(&mdev->tx_napi);
+ tasklet_kill(&mdev->pre_tbtt_tasklet);
+ tasklet_kill(&mdev->tx_tasklet);
+
+ mt76_for_each_q_rx(mdev, i)
+ napi_disable(&mdev->napi[i]);
+
+ mt76x02_dma_reset(dev);
+
+ pci_enable_wake(pdev, pci_choose_state(pdev, state), true);
+ pci_save_state(pdev);
+ err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ if (err)
+ goto restore;
+
+ return 0;
+
+restore:
+ mt76_for_each_q_rx(mdev, i)
+ napi_enable(&mdev->napi[i]);
+ napi_enable(&mdev->tx_napi);
+
+ return err;
+}
+
+static int __maybe_unused
+mt76x2e_resume(struct pci_dev *pdev)
+{
+ struct mt76_dev *mdev = pci_get_drvdata(pdev);
+ int i, err;
+
+ err = pci_set_power_state(pdev, PCI_D0);
+ if (err)
+ return err;
+
+ pci_restore_state(pdev);
+
+ mt76_for_each_q_rx(mdev, i) {
+ napi_enable(&mdev->napi[i]);
+ napi_schedule(&mdev->napi[i]);
+ }
+ napi_enable(&mdev->tx_napi);
+ napi_schedule(&mdev->tx_napi);
+
+ return 0;
+}
+
MODULE_DEVICE_TABLE(pci, mt76pci_device_table);
MODULE_FIRMWARE(MT7662_FIRMWARE);
MODULE_FIRMWARE(MT7662_ROM_PATCH);
@@ -113,6 +167,10 @@ static struct pci_driver mt76pci_driver = {
.id_table = mt76pci_device_table,
.probe = mt76pci_probe,
.remove = mt76pci_remove,
+#ifdef CONFIG_PM
+ .suspend = mt76x2e_suspend,
+ .resume = mt76x2e_resume,
+#endif /* CONFIG_PM */
};
module_pci_driver(mt76pci_driver);
--
2.26.2
>
> Thanks.
>
> --
> Best regards,
> Oleksandr Natalenko (post-factum)
> Principal Software Maintenance Engineer
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply related
* Re: [PATCH iproute2] tc: m_tunnel_key: fix geneve opt output
From: Simon Horman @ 2020-06-18 10:51 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Davide Caratti, lucien.xin, Stephen Hemminger
In-Reply-To: <20200618104420.499155-1-liuhangbin@gmail.com>
On Thu, Jun 18, 2020 at 06:44:20PM +0800, Hangbin Liu wrote:
> Commit f72c3ad00f3b changed the geneve option output from "geneve_opt"
> to "geneve_opts", which may break the program compatibility. Reset
> it back to geneve_opt.
>
> Fixes: f72c3ad00f3b ("tc: m_tunnel_key: add options support for vxlan")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Thanks Hangbin.
I agree that the patch in question did change the name of the option
as you describe, perhaps inadvertently. But I wonder if perhaps this fix
is too simple as the patch mentioned also:
1. Documents the option as geneve_opts
2. Adds vxlan_opts
So this patch invalidates the documentation and creates asymmetry between
the VXLAN and Geneve variants of this feature.
Another problem is that any user of geneve_opts will break.
Perhaps a way out of this mess is to:
1. make geneve_opt an alias for geneve_opts (i.e. two names for the same
thing)
2. Document geneve_opt, possibly marking it as deprecated.
> ---
> tc/m_tunnel_key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
> index bfec9072..0074f744 100644
> --- a/tc/m_tunnel_key.c
> +++ b/tc/m_tunnel_key.c
> @@ -534,7 +534,7 @@ static void tunnel_key_print_geneve_options(struct rtattr *attr)
> struct rtattr *i = RTA_DATA(attr);
> int ii, data_len = 0, offset = 0;
> int rem = RTA_PAYLOAD(attr);
> - char *name = "geneve_opts";
> + char *name = "geneve_opt";
> char strbuf[rem * 2 + 1];
> char data[rem * 2 + 1];
> uint8_t data_r[rem];
> --
> 2.25.4
>
^ permalink raw reply
* Re: [PATCH net v4 0/4] several fixes for indirect flow_blocks offload
From: Simon Horman @ 2020-06-18 10:45 UTC (permalink / raw)
To: wenxu; +Cc: netdev, davem, pablo, vladbu
In-Reply-To: <1592412907-3856-1-git-send-email-wenxu@ucloud.cn>
On Thu, Jun 18, 2020 at 12:55:03AM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> v2:
> patch2: store the cb_priv of representor to the flow_block_cb->indr.cb_priv
> in the driver. And make the correct check with the statments
> this->indr.cb_priv == cb_priv
>
> patch4: del the driver list only in the indriect cleanup callbacks
>
> v3:
> add the cover letter and changlogs.
>
> v4:
> collapsed 1/4, 2/4, 4/4 in v3 to one fix
> Add the prepare patch 1 and 2
>
> This series fixes commit 1fac52da5942 ("net: flow_offload: consolidate
> indirect flow_block infrastructure") that revists the flow_block
> infrastructure.
>
> patch #1 #2：prepare for fix patch #3
> add and use flow_indr_block_cb_alloc/remove function
>
> patch #3: fix flow_indr_dev_unregister path
> If the representor is removed, then identify the indirect flow_blocks
> that need to be removed by the release callback and the port representor
> structure. To identify the port representor structure, a new
> indr.cb_priv field needs to be introduced. The flow_block also needs to
> be removed from the driver list from the cleanup path
>
>
> patch#4 fix block->nooffloaddevcnt warning dmesg log.
> When a indr device add in offload success. The block->nooffloaddevcnt
> should be 0. After the representor go away. When the dir device go away
> the flow_block UNBIND operation with -EOPNOTSUPP which lead the warning
> demesg log.
> The block->nooffloaddevcnt should always count for indr block.
> even the indr block offload successful. The representor maybe
> gone away and the ingress qdisc can work in software mode.
Thanks Wenxu,
Pablo's minor comment wrt patch #1 not withstanding
this series is now looking good to me.
With that fixed feel free to add:
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* [PATCH iproute2] tc: m_tunnel_key: fix geneve opt output
From: Hangbin Liu @ 2020-06-18 10:44 UTC (permalink / raw)
To: netdev
Cc: Davide Caratti, lucien.xin, Simon Horman, Stephen Hemminger,
Hangbin Liu
Commit f72c3ad00f3b changed the geneve option output from "geneve_opt"
to "geneve_opts", which may break the program compatibility. Reset
it back to geneve_opt.
Fixes: f72c3ad00f3b ("tc: m_tunnel_key: add options support for vxlan")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
tc/m_tunnel_key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index bfec9072..0074f744 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -534,7 +534,7 @@ static void tunnel_key_print_geneve_options(struct rtattr *attr)
struct rtattr *i = RTA_DATA(attr);
int ii, data_len = 0, offset = 0;
int rem = RTA_PAYLOAD(attr);
- char *name = "geneve_opts";
+ char *name = "geneve_opt";
char strbuf[rem * 2 + 1];
char data[rem * 2 + 1];
uint8_t data_r[rem];
--
2.25.4
^ permalink raw reply related
* [kbuild] Re: [PATCH 2/5] Huawei BMA: Adding Huawei BMA driver: host_cdev_drv
From: Dan Carpenter @ 2020-06-18 10:28 UTC (permalink / raw)
To: kbuild, yunaixin03610, netdev; +Cc: lkp, kbuild-all, yunaixin
In-Reply-To: <20200615145906.1013-3-yunaixin03610@163.com>
Hi,
url: https://github.com/0day-ci/linux/commits/yunaixin03610-163-com/Adding-Huawei-BMA-drivers/20200616-102318
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a5dc8300df75e8b8384b4c82225f1e4a0b4d9b55
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
cppcheck warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/huawei/bma/edma_drv/bma_pci.c:149:3: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour [shiftTooManyBitsSigned]
REGION_DIR_INPUT + (region & REGION_INDEX_MASK));
^
drivers/net/ethernet/huawei/bma/edma_drv/bma_pci.c:158:55: warning: Shifting signed 32-bit value by 31 bits is undefined behaviour [shiftTooManyBitsSigned]
(void)pci_write_config_dword(pdev, ATU_REGION_CTRL2, REGION_ENABLE);
^
--
>> drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:63:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "lost_count :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:65:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "b2h_int :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:67:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "h2b_int :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:69:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "dma_count :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:71:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "recv_bytes :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:73:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "send_bytes :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:75:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "recv_pkgs :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:77:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "send_pkgs :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:79:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "drop_pkgs :%dn",
^
drivers/net/ethernet/huawei/bma/edma_drv/edma_host.c:81:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
len += sprintf(buf + len, "fail_count :%dn",
^
>> drivers/net/ethernet/huawei/bma/cdev_drv/bma_cdev.c:326:21: warning: Checking if unsigned variable 'count' is less than zero. [unsignedLessThanZero]
if (!data || count <= 0)
^
drivers/net/ethernet/huawei/bma/cdev_drv/bma_cdev.c:346:21: warning: Checking if unsigned variable 'count' is less than zero. [unsignedLessThanZero]
if (!data || count <= 0)
# https://github.com/0day-ci/linux/commit/d0965c4179b69ddd204c1f795f0d6ac080c657af
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d0965c4179b69ddd204c1f795f0d6ac080c657af
vim +122 drivers/net/ethernet/huawei/bma/cdev_drv/bma_cdev.c
d0965c4179b69d yunaixin 2020-06-15 93 static int cdev_param_get_statics(char *buf, const struct kernel_param *kp)
d0965c4179b69d yunaixin 2020-06-15 94 {
d0965c4179b69d yunaixin 2020-06-15 95 int len = 0;
d0965c4179b69d yunaixin 2020-06-15 96 int i = 0;
d0965c4179b69d yunaixin 2020-06-15 97 __kernel_time_t running_time = 0;
d0965c4179b69d yunaixin 2020-06-15 98
d0965c4179b69d yunaixin 2020-06-15 99 if (!buf)
d0965c4179b69d yunaixin 2020-06-15 100 return 0;
buf is a PAGE_SIZE buffer. It can't be NULL.
d0965c4179b69d yunaixin 2020-06-15 101
d0965c4179b69d yunaixin 2020-06-15 102 GET_SYS_SECONDS(running_time);
d0965c4179b69d yunaixin 2020-06-15 103 running_time -= g_cdev_set.init_time;
d0965c4179b69d yunaixin 2020-06-15 104 len += sprintf(buf + len,
d0965c4179b69d yunaixin 2020-06-15 105 "============================CDEV_DRIVER_INFO=======================\n");
d0965c4179b69d yunaixin 2020-06-15 106 len += sprintf(buf + len, "version :%s\n", CDEV_VERSION);
d0965c4179b69d yunaixin 2020-06-15 107
d0965c4179b69d yunaixin 2020-06-15 108 len += sprintf(buf + len, "running_time :%luD %02lu:%02lu:%02lu\n",
d0965c4179b69d yunaixin 2020-06-15 109 running_time / (SECONDS_PER_DAY),
d0965c4179b69d yunaixin 2020-06-15 110 running_time % (SECONDS_PER_DAY) / SECONDS_PER_HOUR,
d0965c4179b69d yunaixin 2020-06-15 111 running_time % SECONDS_PER_HOUR / SECONDS_PER_MINUTE,
d0965c4179b69d yunaixin 2020-06-15 112 running_time % SECONDS_PER_MINUTE);
d0965c4179b69d yunaixin 2020-06-15 113
d0965c4179b69d yunaixin 2020-06-15 114 for (i = 0; i < g_cdev_set.dev_num; i++) {
d0965c4179b69d yunaixin 2020-06-15 115 len += sprintf(buf + len,
d0965c4179b69d yunaixin 2020-06-15 116 "===================================================\n");
I'm very worried that the sprintf loop can overflow the PAGE_SIZE.
Please replace all the sprintf() calls with scnprintf().
len += scnprintf(PAGE_SIZE - len, buf + len, "blah blah");
d0965c4179b69d yunaixin 2020-06-15 117 len += sprintf(buf + len, "name :%s\n",
d0965c4179b69d yunaixin 2020-06-15 118 g_cdev_set.dev_list[i].dev_name);
d0965c4179b69d yunaixin 2020-06-15 119 len +=
d0965c4179b69d yunaixin 2020-06-15 120 sprintf(buf + len, "dev_id :%08x\n",
d0965c4179b69d yunaixin 2020-06-15 121 g_cdev_set.dev_list[i].dev_id);
d0965c4179b69d yunaixin 2020-06-15 @122 len += sprintf(buf + len, "type :%u\n",
d0965c4179b69d yunaixin 2020-06-15 123 g_cdev_set.dev_list[i].type);
d0965c4179b69d yunaixin 2020-06-15 124 len += sprintf(buf + len, "status :%s\n",
d0965c4179b69d yunaixin 2020-06-15 125 g_cdev_set.dev_list[i].s.open_status ==
d0965c4179b69d yunaixin 2020-06-15 126 1 ? "open" : "close");
d0965c4179b69d yunaixin 2020-06-15 127 len += sprintf(buf + len, "send_pkgs :%u\n",
d0965c4179b69d yunaixin 2020-06-15 128 g_cdev_set.dev_list[i].s.send_pkgs);
d0965c4179b69d yunaixin 2020-06-15 129 len +=
d0965c4179b69d yunaixin 2020-06-15 130 sprintf(buf + len, "send_bytes:%u\n",
d0965c4179b69d yunaixin 2020-06-15 131 g_cdev_set.dev_list[i].s.send_bytes);
d0965c4179b69d yunaixin 2020-06-15 132 len += sprintf(buf + len, "send_failed_count:%u\n",
d0965c4179b69d yunaixin 2020-06-15 133 g_cdev_set.dev_list[i].s.send_failed_count);
d0965c4179b69d yunaixin 2020-06-15 134 len += sprintf(buf + len, "recv_pkgs :%u\n",
d0965c4179b69d yunaixin 2020-06-15 135 g_cdev_set.dev_list[i].s.recv_pkgs);
d0965c4179b69d yunaixin 2020-06-15 136 len += sprintf(buf + len, "recv_bytes:%u\n",
d0965c4179b69d yunaixin 2020-06-15 137 g_cdev_set.dev_list[i].s.recv_bytes);
d0965c4179b69d yunaixin 2020-06-15 138 len += sprintf(buf + len, "recv_failed_count:%u\n",
d0965c4179b69d yunaixin 2020-06-15 139 g_cdev_set.dev_list[i].s.recv_failed_count);
d0965c4179b69d yunaixin 2020-06-15 140 }
d0965c4179b69d yunaixin 2020-06-15 141
d0965c4179b69d yunaixin 2020-06-15 142 return len;
d0965c4179b69d yunaixin 2020-06-15 143 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org
^ permalink raw reply
* Re: [PATCH net] geneve: allow changing DF behavior after creation
From: Stefano Brivio @ 2020-06-18 10:26 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev
In-Reply-To: <3b72fc01841507f8439a90f618ef6f6240b9463f.1592473442.git.sd@queasysnail.net>
On Thu, 18 Jun 2020 12:13:22 +0200
Sabrina Dubroca <sd@queasysnail.net> wrote:
> Currently, trying to change the DF parameter of a geneve device does
> nothing:
>
> # ip -d link show geneve1
> 14: geneve1: <snip>
> link/ether <snip>
> geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
> # ip link set geneve1 type geneve id 1 df unset
> # ip -d link show geneve1
> 14: geneve1: <snip>
> link/ether <snip>
> geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
>
> We just need to update the value in geneve_changelink.
>
> Fixes: a025fb5f49ad ("geneve: Allow configuration of DF behaviour")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
> drivers/net/geneve.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 75266580b586..4661ef865807 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1649,6 +1649,7 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
> geneve->collect_md = metadata;
> geneve->use_udp6_rx_checksums = use_udp6_rx_checksums;
> geneve->ttl_inherit = ttl_inherit;
> + geneve->df = df;
I introduced this bug as I didn't notice the asymmetry with VXLAN,
where vxlan_nl2conf() takes care of this for both new links and link
changes.
Here, this block is duplicated in geneve_configure(), which,
somewhat surprisingly given the name, is not called from
geneve_changelink(). Did you consider factoring out (at least) this
block to have it shared?
Either way,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* [PATCH net] geneve: allow changing DF behavior after creation
From: Sabrina Dubroca @ 2020-06-18 10:13 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, Stefano Brivio
Currently, trying to change the DF parameter of a geneve device does
nothing:
# ip -d link show geneve1
14: geneve1: <snip>
link/ether <snip>
geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
# ip link set geneve1 type geneve id 1 df unset
# ip -d link show geneve1
14: geneve1: <snip>
link/ether <snip>
geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
We just need to update the value in geneve_changelink.
Fixes: a025fb5f49ad ("geneve: Allow configuration of DF behaviour")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/geneve.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 75266580b586..4661ef865807 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1649,6 +1649,7 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
geneve->collect_md = metadata;
geneve->use_udp6_rx_checksums = use_udp6_rx_checksums;
geneve->ttl_inherit = ttl_inherit;
+ geneve->df = df;
geneve_unquiesce(geneve, gs4, gs6);
return 0;
--
2.27.0
^ permalink raw reply related
* RE: [PATCH][next] enetc: Use struct_size() helper in kzalloc()
From: Claudiu Manoil @ 2020-06-18 10:04 UTC (permalink / raw)
To: Gustavo A. R. Silva, David S. Miller, Jakub Kicinski
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Gustavo A. R. Silva
In-Reply-To: <20200617185317.GA623@embeddedor>
>-----Original Message-----
>From: Gustavo A. R. Silva <gustavoars@kernel.org>
>Sent: Wednesday, June 17, 2020 9:53 PM
[...]
>Subject: [PATCH][next] enetc: Use struct_size() helper in kzalloc()
>
>Make use of the struct_size() helper instead of an open-coded version
>in order to avoid any potential type mistakes.
>
>This code was detected with the help of Coccinelle and, audited and
>fixed manually.
>
>Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com>
^ permalink raw reply
* Re: [PATCH] net: macb: reject unsupported rgmii delays
From: Russell King - ARM Linux admin @ 2020-06-18 10:01 UTC (permalink / raw)
To: Helmut Grohne
Cc: Nicolas Ferre, David S. Miller, Jakub Kicinski, Palmer Dabbelt,
Paul Walmsley, netdev@vger.kernel.org, Florian Fainelli
In-Reply-To: <20200618090526.GA10165@laureti-dev>
On Thu, Jun 18, 2020 at 11:05:26AM +0200, Helmut Grohne wrote:
> On Thu, Jun 18, 2020 at 10:45:54AM +0200, Russell King - ARM Linux admin wrote:
> > Why do we need that complexity? If we decide that we can allow
> > phy-mode = "rgmii" and introduce new properties to control the
> > delay, then we just need:
> >
> > rgmii-tx-delay-ps = <nnn>;
> > rgmii-rx-delay-ps = <nnn>;
> >
> > specified in the MAC node (to be applied only at the MAC end) or
> > specified in the PHY node (to be applied only at the PHY end.)
> > In the normal case, this would be the standard delay value, but
> > in exceptional cases where supported, the delays can be arbitary.
> > We know there are PHYs out there which allow other delays.
> >
> > This means each end is responsible for parsing these properties in
> > its own node and applying them - or raising an error if they can't
> > be supported.
>
> Thank you. That makes a lot more sense while keeping the (imo) important
> properties of my proposal:
> * It is backwards compatible. These properties override delays
> specified inside phy-mode. Otherwise the vague phy-mode meaning is
> retained.
> * The ambiguity is resolved. It is always clear where delays should be
> configure and the properties properly account for possible PCB
> traces.
>
> It also resolves my original problem. If support for these properties is
> added to macb_main.c, it would simply check that both delays are 0 as
> internal delays are not supported by the hardware. When I would have
> attempted to configure an rx delay, it would have nicely errored out.
I think we'd want a helper or two to do the parsing and return the
delays, something like:
#define PHY_RGMII_DELAY_PS_NONE 0
#define PHY_RGMII_DELAY_PS_STD 1500
/* @np here should be the MAC node */
int of_mac_get_delays(struct device_node *np,
phy_interface_mode interface,
u32 *tx_delay_ps, u32 *rx_delay_ps)
{
*tx_delay_ps = PHY_RGMII_DELAY_PS_NONE;
*rx_delay_ps = PHY_RGMII_DELAY_PS_NONE;
if (!np)
return 0;
if (interface == PHY_INTERFACE_MODE_RGMII) {
of_property_read_u32(np, "rgmii-tx-delay-ps", tx_delay_ps);
of_property_read_u32(np, "rgmii-rx-delay-ps", rx_delay_ps);
}
return 0;
}
/* @np here should be the PHY node */
int of_phy_get_delays(struct device_node *np,
phy_interface_mode interface,
u32 *tx_delay_ps, u32 *rx_delay_ps)
{
switch (interface) {
case PHY_INTERFACE_MODE_RGMII_ID:
*tx_delay_ps = PHY_RGMII_DELAY_PS_STD;
*rx_delay_ps = PHY_RGMII_DELAY_PS_STD;
return 0;
case PHY_INTERFACE_MODE_RGMII_RXID:
*tx_delay_ps = PHY_RGMII_DELAY_PS_NONE;
*rx_delay_ps = PHY_RGMII_DELAY_PS_STD;
return 0;
case PHY_INTERFACE_MODE_RGMII_TXID:
*tx_delay_ps = PHY_RGMII_DELAY_PS_STD;
*rx_delay_ps = PHY_RGMII_DELAY_PS_NONE;
return 0;
default:
return of_mac_get_delays(np, interface,
tx_delay_ps, rx_delay_ps);
}
}
as a first cut - validation left up to the user of these. At least
we then have an easy interface for PHY drivers to use, for example:
static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
{
u32 tx_delay_ps, rx_delay_ps;
int err;
err = of_phy_get_delays(phydev->mdio.dev.of_node,
phydev->interface, &tx_delay_ps,
&rx_delay_ps);
if (err)
return err;
mscr = 0;
if (tx_delay_ps == PHY_RGMII_DELAY_PS_STD)
mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
else if (tx_delay_ps != PHY_RGMII_DELAY_PS_NONE)
/* ... log an error to kernel log */
return -EINVAL;
if (rx_delay_ps == PHY_RGMII_DELAY_PS_STD)
mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
else if (rx_delay_ps != PHY_RGMII_DELAY_PS_NONE)
/* ... log an error to kernel log */
return -EINVAL;
return phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
MII_88E1121_PHY_MSCR_REG,
MII_88E1121_PHY_MSCR_DELAY_MASK, mscr);
}
> How can we achieve wider consensus on this and put it into the dt
> specification? Should there be drivers supporting these first?
Provide an illustration of the idea in code form for consideration? ;)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net] tc-testing: fix geneve options match in tunnel_key unit tests
From: Hangbin Liu @ 2020-06-18 9:31 UTC (permalink / raw)
To: Davide Caratti
Cc: netdev, Pieter Jansen van Vuuren, Lucas Bates, Simon Horman,
David Miller, lucien.xin, Stephen Hemminger
In-Reply-To: <4c1589d4d2866cdfebf12bb32434210532b3b884.camel@redhat.com>
On Thu, Jun 18, 2020 at 10:53:51AM +0200, Davide Caratti wrote:
> On Thu, 2020-06-18 at 16:37 +0800, Hangbin Liu wrote:
> > tc action print "geneve_opts" instead of "geneve_opt".
> > Fix the typo, or we will unable to match correct action output.
> >
>
> hello Hangbin,
>
> > Fixes: cba54f9cf4ec ("tc-testing: add geneve options in tunnel_key unit tests")
>
> this Fixes: tag is suspicious, when a tdc test is added I would expect to
> see it passing. If I well read the code, the problem has been introduced
> in iproute2, with commit
>
> commit f72c3ad00f3b7869e90840d0098a83cb88224892
> Author: Xin Long <lucien.xin@gmail.com>
> Date: Mon Apr 27 18:27:48 2020 +0800
>
> tc: m_tunnel_key: add options support for vxlan
>
>
> that did:
>
> [...]
>
> static void tunnel_key_print_geneve_options(const char *name,
> - struct rtattr *attr)
> +static void tunnel_key_print_geneve_options(struct rtattr *attr)
> {
> struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX + 1];
> struct rtattr *i = RTA_DATA(attr);
> int ii, data_len = 0, offset = 0;
> int rem = RTA_PAYLOAD(attr);
> + char *name = "geneve_opts";
> char strbuf[rem * 2 + 1];
> char data[rem * 2 + 1];
> uint8_t data_r[rem];
> @@ -421,7 +464,7 @@ static void tunnel_key_print_geneve_options(const char *name,
>
> open_json_array(PRINT_JSON, name);
> print_nl();
> - print_string(PRINT_FP, name, "\t%s ", "geneve_opt");
> + print_string(PRINT_FP, name, "\t%s ", name);
Ah, yes, you are right.
>
>
> (just speculating, because I didn't try older versions of iproute2). But if my
> hypothesis is correct, then the fix should be done in iproute2, WDYT?
If you and Stephen are both agree. I'm OK to fix it on iproute2.
Thanks
Hangbin
^ permalink raw reply
* Question about ICMPv6 parameter problem code more than 3
From: 강유건 @ 2020-06-18 9:30 UTC (permalink / raw)
To: davem, kuznet, yoshfuji, netdev; +Cc: 김주연
Hello
I'm testing linux kernels to get ipv6ready logo certificate.
Some test cases require that node send parameter problem with code 3
("IPv6 First Fragment has incomplete IPv6 Header Chain" referred from IANA)
I found that parameter problem codes are defined only from 0 to 2 in
the Linux kernel.
So my question is as follows
Why are the codes on IANA not implemented? Is it under discussion?
If it's being implemented, can you tell me when it will be completed?
Thank you!
- Yugeon Kang
강 유 건 사원
펌킨네트웍스㈜ 개발1팀
08380 서울시 구로구 디지털로31길 20 에이스테크노타워 5차 405호
Direct: 070-4263-9937
Mobile: 010-9887-3517
E-mail: yugun819@pumpkinnet.com
Tel: 02-863-9380, Fax: 02-2109-6675
www.pumpkinnet.co.kr
^ permalink raw reply
* [PATCH net] enetc: Fix HW_VLAN_CTAG_TX|RX toggling
From: Claudiu Manoil @ 2020-06-18 9:16 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller
VLAN tag insertion/extraction offload is correctly
activated at probe time but deactivation of this feature
(i.e. via ethtool) is broken. Toggling works only for
Tx/Rx ring 0 of a PF, and is ignored for the other rings,
including the VF rings.
To fix this, the existing VLAN offload toggling code
was extended to all the rings assigned to a netdevice,
instead of the default ring 0 (likely a leftover from the
early validation days of this feature). And the code was
moved to the common set_features() function to fix toggling
for the VF driver too.
Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
drivers/net/ethernet/freescale/enetc/enetc.c | 26 +++++++++++++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc_hw.h | 16 +++++++--------
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 8 --------
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 298c557..96831f4 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1595,6 +1595,24 @@ static int enetc_set_psfp(struct net_device *ndev, int en)
return 0;
}
+static void enetc_enable_rxvlan(struct net_device *ndev, bool en)
+{
+ struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ int i;
+
+ for (i = 0; i < priv->num_rx_rings; i++)
+ enetc_bdr_enable_rxvlan(&priv->si->hw, i, en);
+}
+
+static void enetc_enable_txvlan(struct net_device *ndev, bool en)
+{
+ struct enetc_ndev_priv *priv = netdev_priv(ndev);
+ int i;
+
+ for (i = 0; i < priv->num_tx_rings; i++)
+ enetc_bdr_enable_txvlan(&priv->si->hw, i, en);
+}
+
int enetc_set_features(struct net_device *ndev,
netdev_features_t features)
{
@@ -1604,6 +1622,14 @@ int enetc_set_features(struct net_device *ndev,
if (changed & NETIF_F_RXHASH)
enetc_set_rss(ndev, !!(features & NETIF_F_RXHASH));
+ if (changed & NETIF_F_HW_VLAN_CTAG_RX)
+ enetc_enable_rxvlan(ndev,
+ !!(features & NETIF_F_HW_VLAN_CTAG_RX));
+
+ if (changed & NETIF_F_HW_VLAN_CTAG_TX)
+ enetc_enable_txvlan(ndev,
+ !!(features & NETIF_F_HW_VLAN_CTAG_TX));
+
if (changed & NETIF_F_HW_TC)
err = enetc_set_psfp(ndev, !!(features & NETIF_F_HW_TC));
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 6314051..ce0d321 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -531,22 +531,22 @@ struct enetc_msg_cmd_header {
/* Common H/W utility functions */
-static inline void enetc_enable_rxvlan(struct enetc_hw *hw, int si_idx,
- bool en)
+static inline void enetc_bdr_enable_rxvlan(struct enetc_hw *hw, int idx,
+ bool en)
{
- u32 val = enetc_rxbdr_rd(hw, si_idx, ENETC_RBMR);
+ u32 val = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
val = (val & ~ENETC_RBMR_VTE) | (en ? ENETC_RBMR_VTE : 0);
- enetc_rxbdr_wr(hw, si_idx, ENETC_RBMR, val);
+ enetc_rxbdr_wr(hw, idx, ENETC_RBMR, val);
}
-static inline void enetc_enable_txvlan(struct enetc_hw *hw, int si_idx,
- bool en)
+static inline void enetc_bdr_enable_txvlan(struct enetc_hw *hw, int idx,
+ bool en)
{
- u32 val = enetc_txbdr_rd(hw, si_idx, ENETC_TBMR);
+ u32 val = enetc_txbdr_rd(hw, idx, ENETC_TBMR);
val = (val & ~ENETC_TBMR_VIH) | (en ? ENETC_TBMR_VIH : 0);
- enetc_txbdr_wr(hw, si_idx, ENETC_TBMR, val);
+ enetc_txbdr_wr(hw, idx, ENETC_TBMR, val);
}
static inline void enetc_set_bdr_prio(struct enetc_hw *hw, int bdr_idx,
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 824d211..4fac57d 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -649,14 +649,6 @@ static int enetc_pf_set_features(struct net_device *ndev,
netdev_features_t changed = ndev->features ^ features;
struct enetc_ndev_priv *priv = netdev_priv(ndev);
- if (changed & NETIF_F_HW_VLAN_CTAG_RX)
- enetc_enable_rxvlan(&priv->si->hw, 0,
- !!(features & NETIF_F_HW_VLAN_CTAG_RX));
-
- if (changed & NETIF_F_HW_VLAN_CTAG_TX)
- enetc_enable_txvlan(&priv->si->hw, 0,
- !!(features & NETIF_F_HW_VLAN_CTAG_TX));
-
if (changed & NETIF_F_HW_VLAN_CTAG_FILTER) {
struct enetc_pf *pf = enetc_si_priv(priv->si);
--
2.7.4
^ permalink raw reply related
* mt7612 suspend/resume issue
From: Oleksandr Natalenko @ 2020-06-18 9:05 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Felix Fietkau, Ryder Lee, Kalle Valo, David S. Miller,
Jakub Kicinski, Matthias Brugger, linux-wireless, netdev,
linux-mediatek, linux-kernel
Hello, Lorenzo et al.
I'm using MT7612 mini-PCIE cards as both AP in a home server and as a client in
a laptop. The AP works perfectly (after some fixing from your side; thanks for
that!), and so does the client modulo it has issues during system resume.
So, the card is installed in my aging Dell Vostro 3360. The system always
suspends fine, but on resume this can happen occasionally:
===
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: MCU message 2 (seq 11) timed out
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: MCU message 30 (seq 12) timed out
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: MCU message 30 (seq 13) timed out
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: Firmware Version: 0.0.00
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: Build: 1
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: Build Time: 201507311614____
čen 15 20:21:07 spock kernel: mt76x2e 0000:01:00.0: Firmware running!
čen 15 20:21:07 spock kernel: ieee80211 phy0: Hardware restart was requested
čen 15 20:21:08 spock kernel: mt76x2e 0000:01:00.0: MCU message 2 (seq 1) timed out
čen 15 20:21:09 spock kernel: mt76x2e 0000:01:00.0: MCU message 30 (seq 2) timed out
čen 15 20:21:10 spock kernel: mt76x2e 0000:01:00.0: MCU message 30 (seq 3) timed out
čen 15 20:21:10 spock kernel: mt76x2e 0000:01:00.0: Firmware Version: 0.0.00
čen 15 20:21:10 spock kernel: mt76x2e 0000:01:00.0: Build: 1
čen 15 20:21:10 spock kernel: mt76x2e 0000:01:00.0: Build Time: 201507311614____
čen 15 20:21:10 spock kernel: mt76x2e 0000:01:00.0: Firmware running!
čen 15 20:21:10 spock kernel: ieee80211 phy0: Hardware restart was requested
čen 15 20:21:11 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 5) timed out
čen 15 20:21:12 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 6) timed out
čen 15 20:21:13 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 7) timed out
čen 15 20:21:14 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 8) timed out
čen 15 20:21:15 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 9) timed out
čen 15 20:21:16 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 10) timed out
čen 15 20:21:17 spock kernel: mt76x2e 0000:01:00.0: MCU message 31 (seq 11) timed out
čen 15 20:21:17 spock kernel: mt76x2e 0000:01:00.0: Firmware Version: 0.0.00
čen 15 20:21:17 spock kernel: mt76x2e 0000:01:00.0: Build: 1
čen 15 20:21:17 spock kernel: mt76x2e 0000:01:00.0: Build Time: 201507311614____
čen 15 20:21:17 spock kernel: mt76x2e 0000:01:00.0: Firmware running!
čen 15 20:21:17 spock kernel: ieee80211 phy0: Hardware restart was requested
čen 15 20:21:18 spock kernel: ------------[ cut here ]------------
čen 15 20:21:18 spock kernel: WARNING: CPU: 3 PID: 11956 at net/mac80211/util.c:2270 ieee80211_reconfig+0x234/0x1700 [mac80211]
čen 15 20:21:18 spock kernel: Modules linked in: pl2303 md4 nls_utf8 cifs dns_resolver fscache libdes cmac ccm bridge stp llc nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables tun nfnetlink msr nls_iso8859_1 nls_cp437 vfat fat snd_hda_codec_hdmi mt76x2e snd_hda_codec_cirrus snd_hda_codec_generic mt76x2_common mt76x02_lib mt76 snd_hda_intel intel_rapl_msr snd_intel_dspcfg mei_hdcp dell_wmi iTCO_wdt mac80211 iTCO_vendor_support intel_rapl_common sparse_keymap wmi_bmof snd_hda_codec x86_pkg_temp_thermal rtsx_usb_ms intel_powerclamp dell_laptop coretemp ledtrig_audio dell_smbios memstick kvm_intel kvm snd_hda_core dell_wmi_descriptor dcdbas snd_hwdep dell_smm_hwmon cfg80211 snd_pcm irqbypass mousedev intel_cstate psmouse joydev intel_uncore intel_rapl_perf input_leds snd_timer i2c_i801 snd mei_me alx rfkill mei libarc4 lpc_ich mdio soundcore battery wmi evdev mac_hid dell_smo8800 ac tcp_bbr crypto_user ip_tables x_tables xfs dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio libcrc32c
čen 15 20:21:18 spock kernel: crc32c_generic dm_crypt hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid rtsx_usb_sdmmc mmc_core rtsx_usb dm_mod crct10dif_pclmul crc32_pclmul crc32c_intel raid10 ghash_clmulni_intel serio_raw atkbd libps2 md_mod aesni_intel crypto_simd cryptd glue_helper xhci_pci xhci_hcd ehci_pci ehci_hcd i8042 serio i915 intel_gtt i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm agpgart
čen 15 20:21:18 spock kernel: CPU: 3 PID: 11956 Comm: kworker/3:1 Not tainted 5.7.0-pf2 #1
čen 15 20:21:18 spock kernel: Hardware name: Dell Inc. Vostro 3360/0F5DWF, BIOS A18 09/25/2013
čen 15 20:21:18 spock kernel: Workqueue: events_freezable ieee80211_restart_work [mac80211]
čen 15 20:21:18 spock kernel: RIP: 0010:ieee80211_reconfig+0x234/0x1700 [mac80211]
čen 15 20:21:18 spock kernel: Code: 83 b8 0b 00 00 83 e0 fd 83 f8 04 74 e6 48 8b 83 90 04 00 00 a8 01 74 db 48 89 de 48 89 ef e8 03 dc fb ff 41 89 c7 85 c0 74 c9 <0f> 0b 48 8b 5b 08 4c 8b 24 24 48 3b 1c 24 75 12 e9 51 fe ff ff 48
čen 15 20:21:18 spock kernel: RSP: 0018:ffffb803c23ffdf0 EFLAGS: 00010286
čen 15 20:21:18 spock kernel: RAX: 00000000fffffff0 RBX: ffff9595a7564900 RCX: 0000000000000008
čen 15 20:21:18 spock kernel: RDX: 0000000000000000 RSI: 0000000000000100 RDI: 0000000000000100
čen 15 20:21:18 spock kernel: RBP: ffff9595a7ec07e0 R08: 0000000000000000 R09: 0000000000000001
čen 15 20:21:18 spock kernel: R10: 0000000000000001 R11: 0000000000000000 R12: ffff9595a7ec18d0
čen 15 20:21:18 spock kernel: R13: 00000000ffffffff R14: 0000000000000000 R15: 00000000fffffff0
čen 15 20:21:18 spock kernel: FS: 0000000000000000(0000) GS:ffff9595af2c0000(0000) knlGS:0000000000000000
čen 15 20:21:18 spock kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
čen 15 20:21:18 spock kernel: CR2: 000055e56d7de000 CR3: 000000042200a001 CR4: 00000000001706e0
čen 15 20:21:18 spock kernel: Call Trace:
čen 15 20:21:18 spock kernel: ieee80211_restart_work+0xb7/0xe0 [mac80211]
čen 15 20:21:18 spock kernel: process_one_work+0x1d4/0x3c0
čen 15 20:21:18 spock kernel: worker_thread+0x228/0x470
čen 15 20:21:18 spock kernel: ? process_one_work+0x3c0/0x3c0
čen 15 20:21:18 spock kernel: kthread+0x19c/0x1c0
čen 15 20:21:18 spock kernel: ? __kthread_init_worker+0x30/0x30
čen 15 20:21:18 spock kernel: ret_from_fork+0x35/0x40
čen 15 20:21:18 spock kernel: ---[ end trace d8e4d40f48014382 ]---
čen 15 20:21:18 spock kernel: ------------[ cut here ]------------
čen 15 20:21:18 spock kernel: wlp1s0: Failed check-sdata-in-driver check, flags: 0x0
čen 15 20:21:18 spock kernel: WARNING: CPU: 3 PID: 11956 at net/mac80211/driver-ops.h:17 drv_remove_interface+0x11f/0x130 [mac80211]
čen 15 20:21:18 spock kernel: Modules linked in: pl2303 md4 nls_utf8 cifs dns_resolver fscache libdes cmac ccm bridge stp llc nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables tun nfnetlink msr nls_iso8859_1 nls_cp437 vfat fat snd_hda_codec_hdmi mt76x2e snd_hda_codec_cirrus snd_hda_codec_generic mt76x2_common mt76x02_lib mt76 snd_hda_intel intel_rapl_msr snd_intel_dspcfg mei_hdcp dell_wmi iTCO_wdt mac80211 iTCO_vendor_support intel_rapl_common sparse_keymap wmi_bmof snd_hda_codec x86_pkg_temp_thermal rtsx_usb_ms intel_powerclamp dell_laptop coretemp ledtrig_audio dell_smbios memstick kvm_intel kvm snd_hda_core dell_wmi_descriptor dcdbas snd_hwdep dell_smm_hwmon cfg80211 snd_pcm irqbypass mousedev intel_cstate psmouse joydev intel_uncore intel_rapl_perf input_leds snd_timer i2c_i801 snd mei_me alx rfkill mei libarc4 lpc_ich mdio soundcore battery wmi evdev mac_hid dell_smo8800 ac tcp_bbr crypto_user ip_tables x_tables xfs dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio libcrc32c
čen 15 20:21:18 spock kernel: crc32c_generic dm_crypt hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid rtsx_usb_sdmmc mmc_core rtsx_usb dm_mod crct10dif_pclmul crc32_pclmul crc32c_intel raid10 ghash_clmulni_intel serio_raw atkbd libps2 md_mod aesni_intel crypto_simd cryptd glue_helper xhci_pci xhci_hcd ehci_pci ehci_hcd i8042 serio i915 intel_gtt i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops cec rc_core drm agpgart
čen 15 20:21:18 spock kernel: CPU: 3 PID: 11956 Comm: kworker/3:1 Tainted: G W 5.7.0-pf2 #1
čen 15 20:21:18 spock kernel: Hardware name: Dell Inc. Vostro 3360/0F5DWF, BIOS A18 09/25/2013
čen 15 20:21:18 spock kernel: Workqueue: events_freezable ieee80211_restart_work [mac80211]
čen 15 20:21:18 spock kernel: RIP: 0010:drv_remove_interface+0x11f/0x130 [mac80211]
čen 15 20:21:18 spock kernel: Code: a0 27 d2 da e9 4b ff ff ff 48 8b 86 78 04 00 00 48 8d b6 98 04 00 00 48 c7 c7 e8 1f f7 c0 48 85 c0 48 0f 45 f0 e8 59 fe db da <0f> 0b 5b 5d 41 5c c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00
čen 15 20:21:18 spock kernel: RSP: 0018:ffffb803c23ffc80 EFLAGS: 00010282
čen 15 20:21:18 spock kernel: RAX: 0000000000000000 RBX: ffff9595a7564900 RCX: 0000000000000000
čen 15 20:21:18 spock kernel: RDX: 0000000000000001 RSI: 0000000000000082 RDI: 00000000ffffffff
čen 15 20:21:18 spock kernel: RBP: ffff9595a7ec1930 R08: 00000000000004b6 R09: 0000000000000001
čen 15 20:21:18 spock kernel: R10: 0000000000000001 R11: 0000000000006f08 R12: ffff9595a7ec1000
čen 15 20:21:18 spock kernel: R13: ffff9595a75654b8 R14: ffff9595a7ec0ca0 R15: ffff9595a7ec07e0
čen 15 20:21:18 spock kernel: FS: 0000000000000000(0000) GS:ffff9595af2c0000(0000) knlGS:0000000000000000
čen 15 20:21:18 spock kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
čen 15 20:21:18 spock kernel: CR2: 000055e56d7de000 CR3: 000000042200a001 CR4: 00000000001706e0
čen 15 20:21:18 spock kernel: Call Trace:
čen 15 20:21:18 spock kernel: ieee80211_do_stop+0x5af/0x8c0 [mac80211]
čen 15 20:21:18 spock kernel: ieee80211_stop+0x16/0x20 [mac80211]
čen 15 20:21:18 spock kernel: __dev_close_many+0xaa/0x120
čen 15 20:21:18 spock kernel: dev_close_many+0xa1/0x2b0
čen 15 20:21:18 spock kernel: dev_close+0x6d/0x90
čen 15 20:21:18 spock kernel: cfg80211_shutdown_all_interfaces+0x71/0xd0 [cfg80211]
čen 15 20:21:18 spock kernel: ieee80211_reconfig+0xa2/0x1700 [mac80211]
čen 15 20:21:18 spock kernel: ieee80211_restart_work+0xb7/0xe0 [mac80211]
čen 15 20:21:18 spock kernel: process_one_work+0x1d4/0x3c0
čen 15 20:21:18 spock kernel: worker_thread+0x228/0x470
čen 15 20:21:18 spock kernel: ? process_one_work+0x3c0/0x3c0
čen 15 20:21:18 spock kernel: kthread+0x19c/0x1c0
čen 15 20:21:18 spock kernel: ? __kthread_init_worker+0x30/0x30
čen 15 20:21:18 spock kernel: ret_from_fork+0x35/0x40
čen 15 20:21:18 spock kernel: ---[ end trace d8e4d40f48014383 ]---
===
The Wi-Fi becomes unusable from this point. If I `modprobe -r` the "mt76x2e" module
after this splat, the system hangs completely.
If the system resumes fine, the resume itself takes quite some time (more than
10 seconds).
I've found a workaround for this, though. It seems the system behaves fine if I
do `modprobe -r mt76x2e` before it goes to sleep, and then `modprobe mt76x2e`
after resume. Also, the resume time improves greatly.
I cannot say if it is some regression or not. I've installed the card
just recently, and used it with v5.7 kernel series only.
Do you have any idea what could go wrong and how to approach the issue?
Thanks.
--
Best regards,
Oleksandr Natalenko (post-factum)
Principal Software Maintenance Engineer
^ permalink raw reply
* Re: [PATCH] net: macb: reject unsupported rgmii delays
From: Helmut Grohne @ 2020-06-18 9:05 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Nicolas Ferre, David S. Miller, Jakub Kicinski, Palmer Dabbelt,
Paul Walmsley, netdev@vger.kernel.org, Florian Fainelli
In-Reply-To: <20200618084554.GY1551@shell.armlinux.org.uk>
On Thu, Jun 18, 2020 at 10:45:54AM +0200, Russell King - ARM Linux admin wrote:
> Why do we need that complexity? If we decide that we can allow
> phy-mode = "rgmii" and introduce new properties to control the
> delay, then we just need:
>
> rgmii-tx-delay-ps = <nnn>;
> rgmii-rx-delay-ps = <nnn>;
>
> specified in the MAC node (to be applied only at the MAC end) or
> specified in the PHY node (to be applied only at the PHY end.)
> In the normal case, this would be the standard delay value, but
> in exceptional cases where supported, the delays can be arbitary.
> We know there are PHYs out there which allow other delays.
>
> This means each end is responsible for parsing these properties in
> its own node and applying them - or raising an error if they can't
> be supported.
Thank you. That makes a lot more sense while keeping the (imo) important
properties of my proposal:
* It is backwards compatible. These properties override delays
specified inside phy-mode. Otherwise the vague phy-mode meaning is
retained.
* The ambiguity is resolved. It is always clear where delays should be
configure and the properties properly account for possible PCB
traces.
It also resolves my original problem. If support for these properties is
added to macb_main.c, it would simply check that both delays are 0 as
internal delays are not supported by the hardware. When I would have
attempted to configure an rx delay, it would have nicely errored out.
How can we achieve wider consensus on this and put it into the dt
specification? Should there be drivers supporting these first?
Helmut
^ permalink raw reply
* Re: [PATCH v4 02/11] fs: Move __scm_install_fd() to __fd_install_received()
From: Christian Brauner @ 2020-06-18 8:56 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Sargun Dhillon, Christian Brauner, David S. Miller,
Christoph Hellwig, Tycho Andersen, Jakub Kicinski, Alexander Viro,
Aleksa Sarai, Matt Denton, Jann Horn, Chris Palmer, Robert Sesek,
Giuseppe Scrivano, Greg Kroah-Hartman, Andy Lutomirski,
Will Drewry, Shuah Khan, netdev, containers, linux-api,
linux-fsdevel, linux-kselftest
In-Reply-To: <20200616032524.460144-3-keescook@chromium.org>
On Mon, Jun 15, 2020 at 08:25:15PM -0700, Kees Cook wrote:
> In preparation for users of the "install a received file" logic outside
> of net/ (pidfd and seccomp), relocate and rename __scm_install_fd() from
> net/core/scm.c to __fd_install_received() in fs/file.c, and provide a
> wrapper named fd_install_received_user(), as future patches will change
> the interface to __fd_install_received().
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> fs/file.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/file.h | 8 ++++++++
> include/net/scm.h | 1 -
> net/compat.c | 2 +-
> net/core/scm.c | 32 +-----------------------------
> 5 files changed, 57 insertions(+), 33 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index abb8b7081d7a..fcfddae0d252 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -11,6 +11,7 @@
> #include <linux/export.h>
> #include <linux/fs.h>
> #include <linux/mm.h>
> +#include <linux/net.h>
> #include <linux/sched/signal.h>
> #include <linux/slab.h>
> #include <linux/file.h>
> @@ -18,6 +19,8 @@
> #include <linux/bitops.h>
> #include <linux/spinlock.h>
> #include <linux/rcupdate.h>
> +#include <net/cls_cgroup.h>
> +#include <net/netprio_cgroup.h>
>
> unsigned int sysctl_nr_open __read_mostly = 1024*1024;
> unsigned int sysctl_nr_open_min = BITS_PER_LONG;
> @@ -931,6 +934,50 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags)
> return err;
> }
>
> +/**
> + * __fd_install_received() - Install received file into file descriptor table
> + *
> + * @fd: fd to install into (if negative, a new fd will be allocated)
> + * @file: struct file that was received from another process
> + * @ufd_required: true to use @ufd for writing fd number to userspace
> + * @ufd: __user pointer to write new fd number to
> + * @o_flags: the O_* flags to apply to the new fd entry
> + *
> + * Installs a received file into the file descriptor table, with appropriate
> + * checks and count updates. Optionally writes the fd number to userspace.
> + *
> + * Returns -ve on error.
> + */
> +int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_flags)
> +{
> + struct socket *sock;
> + int new_fd;
> + int error;
> +
> + error = security_file_receive(file);
> + if (error)
> + return error;
> +
> + new_fd = get_unused_fd_flags(o_flags);
> + if (new_fd < 0)
> + return new_fd;
> +
> + error = put_user(new_fd, ufd);
> + if (error) {
> + put_unused_fd(new_fd);
> + return error;
> + }
> +
> + /* Bump the usage count and install the file. */
> + sock = sock_from_file(file, &error);
> + if (sock) {
> + sock_update_netprioidx(&sock->sk->sk_cgrp_data);
> + sock_update_classid(&sock->sk->sk_cgrp_data);
> + }
> + fd_install(new_fd, get_file(file));
> + return 0;
> +}
> +
> static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags)
> {
> int err = -EBADF;
> diff --git a/include/linux/file.h b/include/linux/file.h
> index 122f80084a3e..fe18a1a0d555 100644
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -91,6 +91,14 @@ extern void put_unused_fd(unsigned int fd);
>
> extern void fd_install(unsigned int fd, struct file *file);
>
> +extern int __fd_install_received(struct file *file, int __user *ufd,
> + unsigned int o_flags);
> +static inline int fd_install_received_user(struct file *file, int __user *ufd,
> + unsigned int o_flags)
> +{
> + return __fd_install_received(file, ufd, o_flags);
> +}
Shouldn't this be the other way around such that
fd_install_received_user() is the workhorse that has a "ufd" argument
and fd_install_received() is the static inline function that doesn't?
extern int fd_install_received_user(struct file *file, int __user *ufd, unsigned int o_flags)
static inline int fd_install_received(struct file *file, unsigned int o_flags)
{
return fd_install_received_user(file, NULL, o_flags);
}
(So I'm on vacation this week some my reviews are selective and spotty
but I promise to be back next week. :))
Christian
> +
> extern void flush_delayed_fput(void);
> extern void __fput_sync(struct file *);
>
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 581a94d6c613..1ce365f4c256 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -37,7 +37,6 @@ struct scm_cookie {
> #endif
> };
>
> -int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags);
> void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm);
> void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm);
> int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm);
> diff --git a/net/compat.c b/net/compat.c
> index 27d477fdcaa0..94f288e8dac5 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -298,7 +298,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm)
> int err = 0, i;
>
> for (i = 0; i < fdmax; i++) {
> - err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags);
> + err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags);
> if (err)
> break;
> }
> diff --git a/net/core/scm.c b/net/core/scm.c
> index 6151678c73ed..df190f1fdd28 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -280,36 +280,6 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter
> }
> EXPORT_SYMBOL(put_cmsg_scm_timestamping);
>
> -int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags)
> -{
> - struct socket *sock;
> - int new_fd;
> - int error;
> -
> - error = security_file_receive(file);
> - if (error)
> - return error;
> -
> - new_fd = get_unused_fd_flags(o_flags);
> - if (new_fd < 0)
> - return new_fd;
> -
> - error = put_user(new_fd, ufd);
> - if (error) {
> - put_unused_fd(new_fd);
> - return error;
> - }
> -
> - /* Bump the usage count and install the file. */
> - sock = sock_from_file(file, &error);
> - if (sock) {
> - sock_update_netprioidx(&sock->sk->sk_cgrp_data);
> - sock_update_classid(&sock->sk->sk_cgrp_data);
> - }
> - fd_install(new_fd, get_file(file));
> - return 0;
> -}
> -
> static int scm_max_fds(struct msghdr *msg)
> {
> if (msg->msg_controllen <= sizeof(struct cmsghdr))
> @@ -336,7 +306,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
> }
>
> for (i = 0; i < fdmax; i++) {
> - err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags);
> + err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags);
> if (err)
> break;
> }
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH 0/6] Add Microchip MCP25XXFD CAN driver
From: Manivannan Sadhasivam @ 2020-06-18 8:55 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: wg, kernel, linux-can, netdev, linux-kernel
In-Reply-To: <fbbca009-3c53-6aa9-94ed-7e9e337c31a4@pengutronix.de>
Hi,
On 0611, Marc Kleine-Budde wrote:
> On 6/10/20 9:44 AM, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds CAN network driver support for Microchip MCP25XXFD CAN
> > Controller with MCP2517FD as the target controller version. This series is
> > mostly inspired (or taken) from the previous iterations posted by Martin Sperl.
> > I've trimmed down the parts which are not necessary for the initial version
> > to ease review. Still the series is relatively huge but I hope to get some
> > reviews (post -rcX ofc!).
> >
> > Link to the origial series posted by Martin:
> > https://www.spinics.net/lists/devicetree/msg284462.html
> >
> > I've not changed the functionality much but done some considerable amount of
> > cleanups and also preserved the authorship of Martin for all the patches he has
> > posted earlier. This series has been tested on 96Boards RB3 platform by myself
> > and Martin has tested the previous version on Rpi3 with external MCP2517FD
> > controller.
>
> I initially started looking at Martin's driver and it was not using several
> modern CAN driver infrastructures. I then posted some cleanup patches but Martin
> was not working on the driver any more. Then I decided to rewrite the driver,
> that is the one I'm hoping to mainline soon.
>
So how should we proceed from here? It is okay for me to work on adding some
features and also fixing the issues you've reported so far. But I want to reach
a consensus before moving forward.
If you think that it makes sense to go with your set of patches, then I need an
estimate on when you'll post the first revision.
> Can you give it a try?
>
> https://github.com/marckleinebudde/linux/commits/v5.6-rpi/mcp25xxfd-20200607-41
>
Sure thing. Will do.
Thanks,
Mani
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung West/Dortmund | Phone: +49-231-2826-924 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH net] tc-testing: fix geneve options match in tunnel_key unit tests
From: Davide Caratti @ 2020-06-18 8:53 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Pieter Jansen van Vuuren, Lucas Bates, Simon Horman, David Miller,
lucien.xin
In-Reply-To: <20200618083705.449619-1-liuhangbin@gmail.com>
On Thu, 2020-06-18 at 16:37 +0800, Hangbin Liu wrote:
> tc action print "geneve_opts" instead of "geneve_opt".
> Fix the typo, or we will unable to match correct action output.
>
hello Hangbin,
> Fixes: cba54f9cf4ec ("tc-testing: add geneve options in tunnel_key unit tests")
this Fixes: tag is suspicious, when a tdc test is added I would expect to
see it passing. If I well read the code, the problem has been introduced
in iproute2, with commit
commit f72c3ad00f3b7869e90840d0098a83cb88224892
Author: Xin Long <lucien.xin@gmail.com>
Date: Mon Apr 27 18:27:48 2020 +0800
tc: m_tunnel_key: add options support for vxlan
that did:
[...]
static void tunnel_key_print_geneve_options(const char *name,
- struct rtattr *attr)
+static void tunnel_key_print_geneve_options(struct rtattr *attr)
{
struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX + 1];
struct rtattr *i = RTA_DATA(attr);
int ii, data_len = 0, offset = 0;
int rem = RTA_PAYLOAD(attr);
+ char *name = "geneve_opts";
char strbuf[rem * 2 + 1];
char data[rem * 2 + 1];
uint8_t data_r[rem];
@@ -421,7 +464,7 @@ static void tunnel_key_print_geneve_options(const char *name,
open_json_array(PRINT_JSON, name);
print_nl();
- print_string(PRINT_FP, name, "\t%s ", "geneve_opt");
+ print_string(PRINT_FP, name, "\t%s ", name);
(just speculating, because I didn't try older versions of iproute2). But if my
hypothesis is correct, then the fix should be done in iproute2, WDYT?
thanks,
--
davide
^ permalink raw reply
* Re: [PATCH] net: macb: reject unsupported rgmii delays
From: Russell King - ARM Linux admin @ 2020-06-18 8:45 UTC (permalink / raw)
To: Helmut Grohne
Cc: Nicolas Ferre, David S. Miller, Jakub Kicinski, Palmer Dabbelt,
Paul Walmsley, netdev@vger.kernel.org, Florian Fainelli
In-Reply-To: <20200618081433.GA22636@laureti-dev>
On Thu, Jun 18, 2020 at 10:14:33AM +0200, Helmut Grohne wrote:
> On Wed, Jun 17, 2020 at 02:08:09PM +0200, Russell King - ARM Linux admin wrote:
> > With a fixed link, we could be in either a MAC-to-PHY or MAC-to-MAC
> > setup; we just don't know. However, we don't have is access to the
> > PHY (if it exists) in the fixed link case to configure it for the
> > delay.
>
> Let me twist that a little: We may have access to the PHY, but we don't
> always have access. When we do have access, we have a separate device
> tree node with another fixed-link and another phy-mode. For fixed-links,
> we specify the phy-mode for each end.
If you have access to the PHY, you shouldn't be using fixed-link. In
any case, there is no support for a fixed-link specification at the
PHY end in the kernel. The PHY binding doc does not allow for this
either.
> > In the MAC-to-MAC RGMII setup, where neither MAC can insert the
> > necessary delay, the only way to have a RGMII conformant link is to
> > have the PCB traces induce the necessary delay. That errs towards
> > PHY_INTERFACE_MODE_RGMII for this case.
>
> Yes.
>
> > However, considering the MAC-to-PHY RGMII fixed link case, where the
> > PHY may not be accessible, and may be configured with the necessary
> > delay, should that case also use PHY_INTERFACE_MODE_RGMII - clearly
> > that would be as wrong as using PHY_INTERFACE_MODE_RGMII_ID would
> > be for the MAC-to-MAC RGMII with PCB-delays case.
>
> If you take into account that the PHY has a separate node with phy-mode
> being rgmii-id, it makes a lot more sense to use rgmii for the phy-mode
> of the MAC. So I don't think it is that clear that doing so is wrong.
The PHY binding document does not allow for this, neither does the
kernel.
> In an earlier discussion Florian Fainelli said:
> https://patchwork.ozlabs.org/project/netdev/patch/20190410005700.31582-19-olteanv@gmail.com/#2149183
> | fixed-link really should denote a MAC to MAC connection so if you have
> | "rgmii-id" on one side, you would expect "rgmii" on the other side
> | (unless PCB traces account for delays, grrr).
>
> For these reasons, I still think that rgmii would be a useful
> description for the fixed-link to PHY connection where the PHY adds the
> delay.
I think Florian is wrong; consider what it means when you have a
fixed-link connection between a MAC and an inaccessible PHY and
the link as a whole is operating in what we would call "rgmii-id"
mode if the PHY were accessible.
Taking Florian's stance, it basically means that DT no longer
describes the hardware, but how we have chosen to interpret the
properties in _one_ specific case in a completely different way
to all the other cases.
> > So, I think a MAC driver should not care about the specific RGMII
> > mode being asked for in any case, and just accept them all.
>
> I would like to agree to this. However, the implication is that when you
> get your delays wrong, your driver silently ignores you and you never
> notice your mistake until you see no traffic passing and wonder why.
So explain to me this aspect of your reasoning:
- If the link is operating in non-fixed-link mode, the rgmii* PHY modes
describe the delay to be applied at the PHY end.
- If the link is operating in fixed-link mode, the rgmii* PHY modes
describe the delay to be applied at the MAC end.
That doesn't make sense, and excludes properly describing a MAC-to-
inaccessible-PHY setup.
It also means that we're having to conditionalise how we deal with
this PHY mode in every single driver, which means that every single
driver is going to end up interpreting it differently, and it's going
to become a buggy mess.
> In this case, I was faced with a PHY that would do rgmii-txid and I
> configured that on the MAC. Unfortunately, macb_main.c didn't tell me
> that it did rgmii-id instead.
The documentation makes it clear that "rgmii-*" (note the hyphen) are
to be applied by the PHY *only*, and not by the MAC.
> > I also think that some of this ought to be put in the documentation
> > as guidance for new implementations.
>
> That seems to be the part where everyone agrees.
>
> Given the state of the discussion, I'm wondering whether this could be
> fixed at a more fundamental level in the device tree bindings.
>
> A number of people (including you) pointed out that retroactively fixing
> the meaning of phy modes does not work and causes pain instead. That
> hints that the only way to fix this is adding new properties. How about
> this?
>
> rgmii-delay-type:
> description:
> Responsibility for adding the rgmii delay
> enum:
> # The remote PHY or MAC to this MAC is responsible for adding the
> # delay.
> - remote
> # The delay is added by neither MAC nor MAC, but using PCB traces
> # instead.
> - pcb
> # The MAC must add the delay.
> - local
Why do we need that complexity? If we decide that we can allow
phy-mode = "rgmii" and introduce new properties to control the
delay, then we just need:
rgmii-tx-delay-ps = <nnn>;
rgmii-rx-delay-ps = <nnn>;
specified in the MAC node (to be applied only at the MAC end) or
specified in the PHY node (to be applied only at the PHY end.)
In the normal case, this would be the standard delay value, but
in exceptional cases where supported, the delays can be arbitary.
We know there are PHYs out there which allow other delays.
This means each end is responsible for parsing these properties in
its own node and applying them - or raising an error if they can't
be supported.
With your "rgmii-delay-type" idea, if this is only specified at the
MAC end, then the PHY code somehow needs to know what that setting is,
which adds a lot of complexity - the PHY code has to go digging for
the MAC node, we may even have to introduce a back-reference from the
PHY node to the MAC node so the PHY can find it. There are MAC drivers
out there where there is one struct device, but multiple net devices,
so digging inside struct net_device to get at the parent struct device,
and then trying to parse "rgmii-delay-type" from the of-node won't work.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH v2] net: macb: undo operations in case of failure
From: Claudiu Beznea @ 2020-06-18 8:37 UTC (permalink / raw)
To: nicolas.ferre, davem, kuba, linux
Cc: antoine.tenart, netdev, linux-kernel, Claudiu Beznea
Undo previously done operation in case macb_phylink_connect()
fails. Since macb_reset_hw() is the 1st undo operation the
napi_exit label was renamed to reset_hw.
Fixes: 7897b071ac3b ("net: macb: convert to phylink")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
Changes in v2:
- corrected fixes SHA1
drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 67933079aeea..257c4920cb88 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2558,7 +2558,7 @@ static int macb_open(struct net_device *dev)
err = macb_phylink_connect(bp);
if (err)
- goto napi_exit;
+ goto reset_hw;
netif_tx_start_all_queues(dev);
@@ -2567,9 +2567,11 @@ static int macb_open(struct net_device *dev)
return 0;
-napi_exit:
+reset_hw:
+ macb_reset_hw(bp);
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue)
napi_disable(&queue->napi);
+ macb_free_consistent(bp);
pm_exit:
pm_runtime_put_sync(&bp->pdev->dev);
return err;
--
2.7.4
^ permalink raw reply related
* [PATCH net] tc-testing: fix geneve options match in tunnel_key unit tests
From: Hangbin Liu @ 2020-06-18 8:37 UTC (permalink / raw)
To: netdev
Cc: Pieter Jansen van Vuuren, Lucas Bates, Simon Horman, David Miller,
Hangbin Liu
tc action print "geneve_opts" instead of "geneve_opt".
Fix the typo, or we will unable to match correct action output.
Fixes: cba54f9cf4ec ("tc-testing: add geneve options in tunnel_key unit tests")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
.../tc-testing/tc-tests/actions/tunnel_key.json | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
index fbeb9197697d..f451915626eb 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
@@ -629,7 +629,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022 index 1",
"expExitCode": "0",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:80:00880022.*index 1",
"matchCount": "1",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -653,7 +653,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022,0408:42:0040007611223344,0111:02:1020304011223344 index 1",
"expExitCode": "0",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022,0408:42:0040007611223344,0111:02:1020304011223344.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:80:00880022,0408:42:0040007611223344,0111:02:1020304011223344.*index 1",
"matchCount": "1",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -677,7 +677,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 824212:80:00880022 index 1",
"expExitCode": "255",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 824212:80:00880022.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 824212:80:00880022.*index 1",
"matchCount": "0",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -701,7 +701,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:4224:00880022 index 1",
"expExitCode": "255",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:4224:00880022.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:4224:00880022.*index 1",
"matchCount": "0",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -725,7 +725,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:4288 index 1",
"expExitCode": "255",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:4288.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:80:4288.*index 1",
"matchCount": "0",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -749,7 +749,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:4288428822 index 1",
"expExitCode": "255",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:4288428822.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:80:4288428822.*index 1",
"matchCount": "0",
"teardown": [
"$TC actions flush action tunnel_key"
@@ -773,7 +773,7 @@
"cmdUnderTest": "$TC actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 dst_port 6081 geneve_opts 0102:80:00880022,0408:42: index 1",
"expExitCode": "255",
"verifyCmd": "$TC actions get action tunnel_key index 1",
- "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opt 0102:80:00880022,0408:42:.*index 1",
+ "matchPattern": "action order [0-9]+: tunnel_key.*set.*src_ip 1.1.1.1.*dst_ip 2.2.2.2.*key_id 42.*dst_port 6081.*geneve_opts 0102:80:00880022,0408:42:.*index 1",
"matchCount": "0",
"teardown": [
"$TC actions flush action tunnel_key"
--
2.25.4
^ permalink raw reply related
* [PATCH net-next v3 2/3] net/mlx5e: Introduce mlx5e_rep_uplink_priv helper
From: xiangxia.m.yue @ 2020-06-18 8:36 UTC (permalink / raw)
To: paulb, saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <20200618083646.59507-1-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Introduce the mlx5e_rep_uplink_priv helper
to make the codes more readable.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
.../ethernet/mellanox/mlx5/core/en/rep/tc.c | 9 ++-------
.../ethernet/mellanox/mlx5/core/en/tc_ct.c | 4 +---
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 9 +++++++++
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 20 +++++--------------
4 files changed, 17 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
index 80713123de5c..e92403a470cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
@@ -507,7 +507,6 @@ static bool mlx5e_restore_tunnel(struct mlx5e_priv *priv, struct sk_buff *skb,
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct tunnel_match_enc_opts enc_opts = {};
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
struct metadata_dst *tun_dst;
struct tunnel_match_key key;
u32 tun_id, enc_opts_id;
@@ -520,9 +519,7 @@ static bool mlx5e_restore_tunnel(struct mlx5e_priv *priv, struct sk_buff *skb,
if (!tun_id)
return true;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
-
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
err = mapping_find(uplink_priv->tunnel_mapping, tun_id, &key);
if (err) {
WARN_ON_ONCE(true);
@@ -588,7 +585,6 @@ bool mlx5e_rep_tc_update_skb(struct mlx5_cqe64 *cqe,
#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
u32 chain = 0, reg_c0, reg_c1, tunnel_id, tuple_id;
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
struct tc_skb_ext *tc_skb_ext;
struct mlx5_eswitch *esw;
struct mlx5e_priv *priv;
@@ -625,8 +621,7 @@ bool mlx5e_rep_tc_update_skb(struct mlx5_cqe64 *cqe,
tuple_id = reg_c1 & TUPLE_ID_MAX;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
if (!mlx5e_tc_ct_restore_flow(uplink_priv, skb, tuple_id))
return false;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
index 430025550fad..71841e7cca88 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
@@ -111,10 +111,8 @@ mlx5_tc_ct_get_ct_priv(struct mlx5e_priv *priv)
{
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
return uplink_priv->ct_priv;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index 1d5669801484..adcd3db0638f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -108,6 +108,15 @@ struct mlx5e_rep_priv *mlx5e_rep_to_rep_priv(struct mlx5_eswitch_rep *rep)
return rep->rep_data[REP_ETH].priv;
}
+static inline struct mlx5_rep_uplink_priv *
+mlx5e_rep_uplink_priv(struct mlx5_eswitch *esw)
+{
+ struct mlx5e_rep_priv *priv;
+
+ priv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
+ return &priv->uplink_priv;
+}
+
struct mlx5e_neigh {
struct net_device *dev;
union {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 05f8df8b53af..b33e40455b51 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1245,12 +1245,10 @@ static void unready_flow_del(struct mlx5e_tc_flow *flow)
static void add_unready_flow(struct mlx5e_tc_flow *flow)
{
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *rpriv;
struct mlx5_eswitch *esw;
esw = flow->priv->mdev->priv.eswitch;
- rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
mutex_lock(&uplink_priv->unready_flows_lock);
unready_flow_add(flow, &uplink_priv->unready_flows);
@@ -1260,12 +1258,10 @@ static void add_unready_flow(struct mlx5e_tc_flow *flow)
static void remove_unready_flow(struct mlx5e_tc_flow *flow)
{
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *rpriv;
struct mlx5_eswitch *esw;
esw = flow->priv->mdev->priv.eswitch;
- rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
mutex_lock(&uplink_priv->unready_flows_lock);
unready_flow_del(flow);
@@ -1937,7 +1933,6 @@ static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
struct flow_match_enc_opts enc_opts_match;
struct tunnel_match_enc_opts tun_enc_opts;
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
struct tunnel_match_key tunnel_key;
bool enc_opts_is_dont_care = true;
u32 tun_id, enc_opts_id = 0;
@@ -1946,8 +1941,7 @@ static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
int err;
esw = priv->mdev->priv.eswitch;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
mlx5e_make_tunnel_match_key(f, filter_dev, &tunnel_key);
err = mapping_add(uplink_priv->tunnel_mapping, &tunnel_key, &tun_id);
@@ -2012,13 +2006,11 @@ static int mlx5e_lookup_flow_tunnel_id(struct mlx5e_priv *priv,
u32 *tun_id)
{
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
struct tunnel_match_key tunnel_key;
struct mlx5_eswitch *esw;
esw = priv->mdev->priv.eswitch;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
mlx5e_make_tunnel_match_key(f, filter_dev, &tunnel_key);
return mapping_find_by_data(uplink_priv->tunnel_mapping, &tunnel_key, tun_id);
@@ -2029,12 +2021,10 @@ static void mlx5e_put_flow_tunnel_id(struct mlx5e_tc_flow *flow)
u32 enc_opts_id = flow->tunnel_id & ENC_OPTS_BITS_MASK;
u32 tun_id = flow->tunnel_id >> ENC_OPTS_BITS;
struct mlx5_rep_uplink_priv *uplink_priv;
- struct mlx5e_rep_priv *uplink_rpriv;
struct mlx5_eswitch *esw;
esw = flow->priv->mdev->priv.eswitch;
- uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
- uplink_priv = &uplink_rpriv->uplink_priv;
+ uplink_priv = mlx5e_rep_uplink_priv(esw);
if (tun_id)
mapping_remove(uplink_priv->tunnel_mapping, tun_id);
--
2.26.1
^ permalink raw reply related
* [PATCH net-next v3 3/3] net/mlx5e: Fix the code style
From: xiangxia.m.yue @ 2020-06-18 8:36 UTC (permalink / raw)
To: paulb, saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <20200618083646.59507-1-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 006807e04eda..795dda1a5e8a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -533,7 +533,7 @@ static bool mlx5e_rep_has_offload_stats(const struct net_device *dev, int attr_i
{
switch (attr_id) {
case IFLA_OFFLOAD_XSTATS_CPU_HIT:
- return true;
+ return true;
}
return false;
--
2.26.1
^ permalink raw reply related
* [PATCH net-next v3 1/3] net/mlx5e: Implicitly decap the tunnel packet when necessary
From: xiangxia.m.yue @ 2020-06-18 8:36 UTC (permalink / raw)
To: paulb, saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
The commit 0a7fcb78cc21 ("net/mlx5e: Support inner header rewrite with
goto action"), will decapsulate the tunnel packets if there is a goto
action in chain 0. But in some case, we don't want do that, for example:
$ tc filter add dev $VXLAN protocol ip parent ffff: prio 1 chain 0 \
flower enc_dst_ip 2.2.2.100 enc_dst_port 4789 \
action goto chain 2
$ tc filter add dev $VXLAN protocol ip parent ffff: prio 1 chain 2 \
flower dst_mac 00:11:22:33:44:55 enc_src_ip 2.2.2.200 \
enc_dst_ip 2.2.2.100 enc_dst_port 4789 enc_key_id 100 \
action tunnel_key unset action mirred egress redirect dev enp130s0f0_0
$ tc filter add dev $VXLAN protocol ip parent ffff: prio 1 chain 2 \
flower dst_mac 00:11:22:33:44:66 enc_src_ip 2.2.2.200 \
enc_dst_ip 2.2.2.100 enc_dst_port 4789 enc_key_id 200 \
action tunnel_key unset action mirred egress redirect dev enp130s0f0_1
If there are pedit and goto actions, do the decapsulate and id mapping action.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
.../ethernet/mellanox/mlx5/core/en/mapping.c | 24 ++++
.../ethernet/mellanox/mlx5/core/en/mapping.h | 1 +
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 109 ++++++++++++------
3 files changed, 99 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.c b/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.c
index ea321e528749..90306dde6b60 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.c
@@ -74,6 +74,30 @@ int mapping_add(struct mapping_ctx *ctx, void *data, u32 *id)
return err;
}
+int mapping_find_by_data(struct mapping_ctx *ctx, void *data, u32 *id)
+{
+ struct mapping_item *mi;
+ u32 hash_key;
+
+ mutex_lock(&ctx->lock);
+
+ hash_key = jhash(data, ctx->data_size, 0);
+ hash_for_each_possible(ctx->ht, mi, node, hash_key) {
+ if (!memcmp(data, mi->data, ctx->data_size))
+ goto found;
+ }
+
+ mutex_unlock(&ctx->lock);
+ return -ENOENT;
+
+found:
+ if (id)
+ *id = mi->id;
+
+ mutex_unlock(&ctx->lock);
+ return 0;
+}
+
static void mapping_remove_and_free(struct mapping_ctx *ctx,
struct mapping_item *mi)
{
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.h b/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.h
index 285525cc5470..af501c9796b7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/mapping.h
@@ -9,6 +9,7 @@ struct mapping_ctx;
int mapping_add(struct mapping_ctx *ctx, void *data, u32 *id);
int mapping_remove(struct mapping_ctx *ctx, u32 id);
int mapping_find(struct mapping_ctx *ctx, u32 id, void *data);
+int mapping_find_by_data(struct mapping_ctx *ctx, void *data, u32 *id);
/* mapping uses an xarray to map data to ids in add(), and for find().
* For locking, it uses a internal xarray spin lock for add()/remove(),
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 7fc84f58e28a..05f8df8b53af 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1836,7 +1836,8 @@ static void mlx5e_tc_del_flow(struct mlx5e_priv *priv,
}
}
-static int flow_has_tc_fwd_action(struct flow_cls_offload *f)
+static int flow_has_tc_action(struct flow_cls_offload *f,
+ enum flow_action_id action)
{
struct flow_rule *rule = flow_cls_offload_flow_rule(f);
struct flow_action *flow_action = &rule->action;
@@ -1844,12 +1845,8 @@ static int flow_has_tc_fwd_action(struct flow_cls_offload *f)
int i;
flow_action_for_each(i, act, flow_action) {
- switch (act->id) {
- case FLOW_ACTION_GOTO:
+ if (act->id == action)
return true;
- default:
- continue;
- }
}
return false;
@@ -1901,10 +1898,37 @@ enc_opts_is_dont_care_or_full_match(struct mlx5e_priv *priv,
sizeof(*__dst));\
})
+static void mlx5e_make_tunnel_match_key(struct flow_cls_offload *f,
+ struct net_device *filter_dev,
+ struct tunnel_match_key *tunnel_key)
+{
+ struct flow_rule *rule = flow_cls_offload_flow_rule(f);
+
+ memset(tunnel_key, 0, sizeof(*tunnel_key));
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_CONTROL,
+ &tunnel_key->enc_control);
+ if (tunnel_key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS)
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS,
+ &tunnel_key->enc_ipv4);
+ else
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS,
+ &tunnel_key->enc_ipv6);
+
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IP, &tunnel_key->enc_ip);
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_PORTS,
+ &tunnel_key->enc_tp);
+ COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_KEYID,
+ &tunnel_key->enc_key_id);
+
+ tunnel_key->filter_ifindex = filter_dev->ifindex;
+}
+
static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
struct mlx5e_tc_flow *flow,
struct flow_cls_offload *f,
- struct net_device *filter_dev)
+ struct net_device *filter_dev,
+ bool sets_mapping,
+ bool needs_mapping)
{
struct flow_rule *rule = flow_cls_offload_flow_rule(f);
struct netlink_ext_ack *extack = f->common.extack;
@@ -1925,22 +1949,7 @@ static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
uplink_priv = &uplink_rpriv->uplink_priv;
- memset(&tunnel_key, 0, sizeof(tunnel_key));
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_CONTROL,
- &tunnel_key.enc_control);
- if (tunnel_key.enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS)
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS,
- &tunnel_key.enc_ipv4);
- else
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS,
- &tunnel_key.enc_ipv6);
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_IP, &tunnel_key.enc_ip);
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_PORTS,
- &tunnel_key.enc_tp);
- COPY_DISSECTOR(rule, FLOW_DISSECTOR_KEY_ENC_KEYID,
- &tunnel_key.enc_key_id);
- tunnel_key.filter_ifindex = filter_dev->ifindex;
-
+ mlx5e_make_tunnel_match_key(f, filter_dev, &tunnel_key);
err = mapping_add(uplink_priv->tunnel_mapping, &tunnel_key, &tun_id);
if (err)
return err;
@@ -1970,10 +1979,10 @@ static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
mask = enc_opts_id ? TUNNEL_ID_MASK :
(TUNNEL_ID_MASK & ~ENC_OPTS_BITS_MASK);
- if (attr->chain) {
+ if (needs_mapping) {
mlx5e_tc_match_to_reg_match(&attr->parse_attr->spec,
TUNNEL_TO_REG, value, mask);
- } else {
+ } else if (sets_mapping) {
mod_hdr_acts = &attr->parse_attr->mod_hdr_acts;
err = mlx5e_tc_match_to_reg_set(priv->mdev,
mod_hdr_acts,
@@ -1996,6 +2005,25 @@ static int mlx5e_get_flow_tunnel_id(struct mlx5e_priv *priv,
return err;
}
+static int mlx5e_lookup_flow_tunnel_id(struct mlx5e_priv *priv,
+ struct mlx5e_tc_flow *flow,
+ struct flow_cls_offload *f,
+ struct net_device *filter_dev,
+ u32 *tun_id)
+{
+ struct mlx5_rep_uplink_priv *uplink_priv;
+ struct mlx5e_rep_priv *uplink_rpriv;
+ struct tunnel_match_key tunnel_key;
+ struct mlx5_eswitch *esw;
+
+ esw = priv->mdev->priv.eswitch;
+ uplink_rpriv = mlx5_eswitch_get_uplink_priv(esw, REP_ETH);
+ uplink_priv = &uplink_rpriv->uplink_priv;
+
+ mlx5e_make_tunnel_match_key(f, filter_dev, &tunnel_key);
+ return mapping_find_by_data(uplink_priv->tunnel_mapping, &tunnel_key, tun_id);
+}
+
static void mlx5e_put_flow_tunnel_id(struct mlx5e_tc_flow *flow)
{
u32 enc_opts_id = flow->tunnel_id & ENC_OPTS_BITS_MASK;
@@ -2057,13 +2085,19 @@ static int parse_tunnel_attr(struct mlx5e_priv *priv,
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct netlink_ext_ack *extack = f->common.extack;
bool needs_mapping, sets_mapping;
+ bool pedit_action;
int err;
if (!mlx5e_is_eswitch_flow(flow))
return -EOPNOTSUPP;
- needs_mapping = !!flow->esw_attr->chain;
- sets_mapping = !flow->esw_attr->chain && flow_has_tc_fwd_action(f);
+ pedit_action = flow_has_tc_action(f, FLOW_ACTION_MANGLE) ||
+ flow_has_tc_action(f, FLOW_ACTION_ADD);
+ sets_mapping = pedit_action &&
+ flow_has_tc_action(f, FLOW_ACTION_GOTO);
+ needs_mapping = !!flow->esw_attr->chain &&
+ !mlx5e_lookup_flow_tunnel_id(priv, flow, f,
+ filter_dev, NULL);
*match_inner = !needs_mapping;
if ((needs_mapping || sets_mapping) &&
@@ -2075,7 +2109,7 @@ static int parse_tunnel_attr(struct mlx5e_priv *priv,
return -EOPNOTSUPP;
}
- if (!flow->esw_attr->chain) {
+ if (*match_inner) {
err = mlx5e_tc_tun_parse(filter_dev, priv, spec, f,
match_level);
if (err) {
@@ -2085,18 +2119,20 @@ static int parse_tunnel_attr(struct mlx5e_priv *priv,
"Failed to parse tunnel attributes");
return err;
}
-
- /* With mpls over udp we decapsulate using packet reformat
- * object
- */
- if (!netif_is_bareudp(filter_dev))
- flow->esw_attr->action |= MLX5_FLOW_CONTEXT_ACTION_DECAP;
}
+ /* With mpls over udp we decapsulate using packet reformat
+ * object
+ */
+ if (!netif_is_bareudp(filter_dev) &&
+ sets_mapping && !needs_mapping)
+ flow->esw_attr->action |= MLX5_FLOW_CONTEXT_ACTION_DECAP;
+
if (!needs_mapping && !sets_mapping)
return 0;
- return mlx5e_get_flow_tunnel_id(priv, flow, f, filter_dev);
+ return mlx5e_get_flow_tunnel_id(priv, flow, f, filter_dev,
+ sets_mapping, needs_mapping);
}
static void *get_match_inner_headers_criteria(struct mlx5_flow_spec *spec)
@@ -4309,6 +4345,9 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
attr->action |= MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
}
+ if (decap)
+ attr->action |= MLX5_FLOW_CONTEXT_ACTION_DECAP;
+
if (!(attr->action &
(MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | MLX5_FLOW_CONTEXT_ACTION_DROP))) {
NL_SET_ERR_MSG_MOD(extack,
--
2.26.1
^ permalink raw reply related
* RE: [PATCH v4 03/11] fs: Add fd_install_received() wrapper for __fd_install_received()
From: David Laight @ 2020-06-18 8:19 UTC (permalink / raw)
To: 'Kees Cook'
Cc: linux-kernel@vger.kernel.org, Sargun Dhillon, Christian Brauner,
David S. Miller, Christoph Hellwig, Tycho Andersen,
Jakub Kicinski, Alexander Viro, Aleksa Sarai, Matt Denton,
Jann Horn, Chris Palmer, Robert Sesek, Giuseppe Scrivano,
Greg Kroah-Hartman, Andy Lutomirski, Will Drewry, Shuah Khan,
netdev@vger.kernel.org, containers@lists.linux-foundation.org,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kselftest@vger.kernel.org
In-Reply-To: <202006171141.4DA1174979@keescook>
From: Kees Cook
> Sent: 17 June 2020 20:58
> On Wed, Jun 17, 2020 at 03:35:20PM +0000, David Laight wrote:
> > From: Kees Cook
> > > Sent: 16 June 2020 04:25
> > >
> > > For both pidfd and seccomp, the __user pointer is not used. Update
> > > __fd_install_received() to make writing to ufd optional. (ufd
> > > itself cannot checked for NULL because this changes the SCM_RIGHTS
> > > interface behavior.) In these cases, the new fd needs to be returned
> > > on success. Update the existing callers to handle it. Add new wrapper
> > > fd_install_received() for pidfd and seccomp that does not use the ufd
> > > argument.
> > ...>
> > > static inline int fd_install_received_user(struct file *file, int __user *ufd,
> > > unsigned int o_flags)
> > > {
> > > - return __fd_install_received(file, ufd, o_flags);
> > > + return __fd_install_received(file, true, ufd, o_flags);
> > > +}
> >
> > Can you get rid of the 'return user' parameter by adding
> > if (!ufd) return -EFAULT;
> > to the above wrapper, then checking for NULL in the function?
> >
> > Or does that do the wrong horrid things in the fail path?
>
> Oh, hm. No, that shouldn't break the failure path, since everything gets
> unwound in __fd_install_received if the ufd write fails.
>
> Effectively this (I'll chop it up into the correct patches):
Yep, that's what i was thinking...
Personally I'm not sure that it matters whether the file is left
attached to a process fd when the copy_to_user() fails.
The kernel data structures are consistent either way.
So sane code relies on catching SIGSEGV, fixing thigs up,
and carrying on.
(IIRC the original /bin/sh code called sbrk() in its SIGSEGV
handler instead of doing the limit check in malloc()!)
The important error path is 'failing to get an fd number'.
In that case the caller needs to keep the 'file *' or close it.
I've not looked at the code, but I wonder if you need to pass
the 'file *' by reference so that you can consume it (write NULL)
and return an error.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] net: macb: reject unsupported rgmii delays
From: Helmut Grohne @ 2020-06-18 8:14 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Nicolas Ferre, David S. Miller, Jakub Kicinski, Palmer Dabbelt,
Paul Walmsley, netdev@vger.kernel.org, Florian Fainelli
In-Reply-To: <20200617120809.GS1551@shell.armlinux.org.uk>
On Wed, Jun 17, 2020 at 02:08:09PM +0200, Russell King - ARM Linux admin wrote:
> With a fixed link, we could be in either a MAC-to-PHY or MAC-to-MAC
> setup; we just don't know. However, we don't have is access to the
> PHY (if it exists) in the fixed link case to configure it for the
> delay.
Let me twist that a little: We may have access to the PHY, but we don't
always have access. When we do have access, we have a separate device
tree node with another fixed-link and another phy-mode. For fixed-links,
we specify the phy-mode for each end.
> In the MAC-to-MAC RGMII setup, where neither MAC can insert the
> necessary delay, the only way to have a RGMII conformant link is to
> have the PCB traces induce the necessary delay. That errs towards
> PHY_INTERFACE_MODE_RGMII for this case.
Yes.
> However, considering the MAC-to-PHY RGMII fixed link case, where the
> PHY may not be accessible, and may be configured with the necessary
> delay, should that case also use PHY_INTERFACE_MODE_RGMII - clearly
> that would be as wrong as using PHY_INTERFACE_MODE_RGMII_ID would
> be for the MAC-to-MAC RGMII with PCB-delays case.
If you take into account that the PHY has a separate node with phy-mode
being rgmii-id, it makes a lot more sense to use rgmii for the phy-mode
of the MAC. So I don't think it is that clear that doing so is wrong.
In an earlier discussion Florian Fainelli said:
https://patchwork.ozlabs.org/project/netdev/patch/20190410005700.31582-19-olteanv@gmail.com/#2149183
| fixed-link really should denote a MAC to MAC connection so if you have
| "rgmii-id" on one side, you would expect "rgmii" on the other side
| (unless PCB traces account for delays, grrr).
For these reasons, I still think that rgmii would be a useful
description for the fixed-link to PHY connection where the PHY adds the
delay.
> So, I think a MAC driver should not care about the specific RGMII
> mode being asked for in any case, and just accept them all.
I would like to agree to this. However, the implication is that when you
get your delays wrong, your driver silently ignores you and you never
notice your mistake until you see no traffic passing and wonder why.
In this case, I was faced with a PHY that would do rgmii-txid and I
configured that on the MAC. Unfortunately, macb_main.c didn't tell me
that it did rgmii-id instead.
> I also think that some of this ought to be put in the documentation
> as guidance for new implementations.
That seems to be the part where everyone agrees.
Given the state of the discussion, I'm wondering whether this could be
fixed at a more fundamental level in the device tree bindings.
A number of people (including you) pointed out that retroactively fixing
the meaning of phy modes does not work and causes pain instead. That
hints that the only way to fix this is adding new properties. How about
this?
rgmii-delay-type:
description:
Responsibility for adding the rgmii delay
enum:
# The remote PHY or MAC to this MAC is responsible for adding the
# delay.
- remote
# The delay is added by neither MAC nor MAC, but using PCB traces
# instead.
- pcb
# The MAC must add the delay.
- local
rgmii-rx-delay:
# Responsibility for RX delay. Delay specification in the phy-mode is
# ignored when this is present.
$ref: "#/properties/rgmii-delay-type"
rgmii-tx-delay:
# Responsibility for TX delay. Delay specification in the phy-mode is
# ignored when this is present.
$ref: "#/properties/rgmii-delay-type"
The naming is up to discussion, but I think you get the idea. The core
properties of this proposal are:
* It does not break existing device trees.
* It completely resolves the present ambiguity.
The major downside is that you never know whether your driver supports
such delay specifications already.
Helmut
^ 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