* [PATCH] net: geneve: Fix a possible null-pointer dereference in geneve_link_config()
From: Jia-Ju Bai @ 2019-07-29 10:26 UTC (permalink / raw)
To: davem, sbrivio, sd, liuhangbin, jbenc, dsahern, natechancellor,
tglx
Cc: netdev, linux-kernel, Jia-Ju Bai
In geneve_link_config(), there is an if statement on line 1524 to check
whether rt is NULL:
if (rt && rt->dst.dev)
When rt is NULL, it is used on line 1526:
ip6_rt_put(rt)
dst_release(&rt->dst);
Thus, a possible null-pointer dereference may occur.
To fix this bug, ip6_rt_put(rt) is called when rt is not NULL.
This bug is found by a static analysis tool STCheck written by us.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/geneve.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index cb2ea8facd8d..a47a1b31b166 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1521,9 +1521,10 @@ static void geneve_link_config(struct net_device *dev,
rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0,
NULL, 0);
- if (rt && rt->dst.dev)
+ if (rt && rt->dst.dev) {
ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
- ip6_rt_put(rt);
+ ip6_rt_put(rt);
+ }
break;
}
#endif
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] net: geneve: Fix a possible null-pointer dereference in geneve_link_config()
From: Jiri Benc @ 2019-07-29 10:30 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, sbrivio, sd, liuhangbin, dsahern, natechancellor, tglx,
netdev, linux-kernel
In-Reply-To: <20190729102611.2338-1-baijiaju1990@gmail.com>
On Mon, 29 Jul 2019 18:26:11 +0800, Jia-Ju Bai wrote:
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1521,9 +1521,10 @@ static void geneve_link_config(struct net_device *dev,
> rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0,
> NULL, 0);
>
> - if (rt && rt->dst.dev)
> + if (rt && rt->dst.dev) {
> ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
> - ip6_rt_put(rt);
> + ip6_rt_put(rt);
> + }
> break;
> }
> #endif
Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev
being NULL? You'd leak the reference in such case.
Jiri
^ permalink raw reply
* Re: [PATCH net-next 08/11] net: hns3: add interrupt affinity support for misc interrupt
From: Yunsheng Lin @ 2019-07-29 10:37 UTC (permalink / raw)
To: Salil Mehta, tanhuazhong, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Zhuangyuzeng (Yisen), Linuxarm, lipeng (Y)
In-Reply-To: <fa3dbcce9e22453fb1ef111fd107d20f@huawei.com>
On 2019/7/29 17:18, Salil Mehta wrote:
>> From: tanhuazhong
>> Sent: Wednesday, July 24, 2019 4:19 AM
>> To: davem@davemloft.net
>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Salil Mehta
>> <salil.mehta@huawei.com>; Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>;
>> Linuxarm <linuxarm@huawei.com>; linyunsheng <linyunsheng@huawei.com>; lipeng
>> (Y) <lipeng321@huawei.com>; tanhuazhong <tanhuazhong@huawei.com>
>> Subject: [PATCH net-next 08/11] net: hns3: add interrupt affinity support for
>> misc interrupt
>>
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>>
>> The misc interrupt is used to schedule the reset and mailbox
>> subtask, and a 1 sec timer is used to schedule the service
>> subtask, which does periodic work.
>>
>> This patch sets the above three subtask's affinity using the
>> misc interrupt' affinity.
>>
>> Also this patch setups a affinity notify for misc interrupt to
>> allow user to change the above three subtask's affinity.
>>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>> Signed-off-by: Peng Li <lipeng321@huawei.com>
>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
>> ---
>> .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 59
>> ++++++++++++++++++++--
>> .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 4 ++
>> 2 files changed, 59 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> index f345095..fe45986 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> @@ -1270,6 +1270,12 @@ static int hclge_configure(struct hclge_dev *hdev)
>>
>> hclge_init_kdump_kernel_config(hdev);
>>
>> + /* Set the init affinity based on pci func number */
>> + i = cpumask_weight(cpumask_of_node(dev_to_node(&hdev->pdev->dev)));
>> + i = i ? PCI_FUNC(hdev->pdev->devfn) % i : 0;
>> + cpumask_set_cpu(cpumask_local_spread(i, dev_to_node(&hdev->pdev->dev)),
>> + &hdev->affinity_mask);
>> +
>> return ret;
>> }
>>
>> @@ -2502,14 +2508,16 @@ static void hclge_mbx_task_schedule(struct hclge_dev
>> *hdev)
>> {
>> if (!test_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state) &&
>> !test_and_set_bit(HCLGE_STATE_MBX_SERVICE_SCHED, &hdev->state))
>> - schedule_work(&hdev->mbx_service_task);
>> + queue_work_on(cpumask_first(&hdev->affinity_mask), system_wq,
>
>
> Why we have to use system Work Queue here? This could adversely affect
> other work scheduled not related to the HNS3 driver. Mailbox is internal
> to the driver and depending upon utilization of the mbx channel(which could
> be heavy if many VMs are running), this might stress other system tasks
> as well. Have we thought of this?
If I understand the CMWQ correctly, it is better to reuse the system Work
Queue when the work queue needed by HNS3 driver shares the same property of
system Work Queue, because Concurrency Managed Workqueue mechnism has ensured
they have the same worker pool if they share the same property.
Some driver(i40e) allocates a work queue with WQ_MEM_RECLAIM, I am not sure
what is the usecase which requires the WQ_MEM_RECLAIM.
Anyway, If the HNS3 need to allocate a new work queue, we need to
figure out the usecase first.
>
>
>
>> + &hdev->mbx_service_task);
>> }
>>
>> static void hclge_reset_task_schedule(struct hclge_dev *hdev)
>> {
>> if (!test_bit(HCLGE_STATE_REMOVING, &hdev->state) &&
>> !test_and_set_bit(HCLGE_STATE_RST_SERVICE_SCHED, &hdev->state))
>> - schedule_work(&hdev->rst_service_task);
>> + queue_work_on(cpumask_first(&hdev->affinity_mask), system_wq,
>> + &hdev->rst_service_task);
>> }
>>
>> static void hclge_task_schedule(struct hclge_dev *hdev)
>> @@ -2517,7 +2525,8 @@ static void hclge_task_schedule(struct hclge_dev *hdev)
>> if (!test_bit(HCLGE_STATE_DOWN, &hdev->state) &&
>> !test_bit(HCLGE_STATE_REMOVING, &hdev->state) &&
>> !test_and_set_bit(HCLGE_STATE_SERVICE_SCHED, &hdev->state))
>> - (void)schedule_work(&hdev->service_task);
>> + queue_work_on(cpumask_first(&hdev->affinity_mask), system_wq,
>
> Same here.
>
>
> Salil.
>
> .
>
^ permalink raw reply
* DSA Rate Limiting in 88E6390
From: Benjamin Beckmeyer @ 2019-07-29 10:30 UTC (permalink / raw)
To: netdev
Hi all,
is there a possibility to set the rate limiting for the 6390 with linux tools?
I've seen that the driver will init all to zero, so rate limiting is disabled,
but there is no solution for it in the ip tool?
The only thing I found for rate limiting is the tc tool, but I guess this is
only a software solution?
Furthermore, does exist a table or a tutorial which functions DSA supports?
The background is that we are using the DSDT driver (in future maybe the UMSD
driver) but we would like to switch to the in kernel DSA entirely. And our
software is using some of the DSDT functions, so I have to find an
alternative to these functions.
Thanks in advance.
Best regards,
Benjamin
^ permalink raw reply
* Re: [PATCH] net: geneve: Fix a possible null-pointer dereference in geneve_link_config()
From: Jiri Benc @ 2019-07-29 10:52 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, sbrivio, sd, liuhangbin, dsahern, natechancellor, tglx,
netdev, linux-kernel
In-Reply-To: <20190729123055.5a7ba416@redhat.com>
On Mon, 29 Jul 2019 12:30:55 +0200, Jiri Benc wrote:
> Are you sure rt6_lookup can never return a non-NULL rt with rt->dst.dev
> being NULL? You'd leak the reference in such case.
In fact, you're introducing a bug, not fixing one. ip6_rt_put does
accept NULL parameter. And it seems you already have been told that?
Nacked-by: Jiri Benc <jbenc@redhat.com>
Jiri
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jon Hunter @ 2019-07-29 10:55 UTC (permalink / raw)
To: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org
Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai,
Maxime Coquelin, linux-tegra, Giuseppe Cavallaro, Robin Murphy,
David S . Miller
In-Reply-To: <MN2PR12MB327907D4A6FB378AC989571AD3DD0@MN2PR12MB3279.namprd12.prod.outlook.com>
On 29/07/2019 09:16, Jose Abreu wrote:
> From: Jose Abreu <joabreu@synopsys.com>
> Date: Jul/27/2019, 16:56:37 (UTC+00:00)
>
>> From: Jon Hunter <jonathanh@nvidia.com>
>> Date: Jul/26/2019, 15:11:00 (UTC+00:00)
>>
>>>
>>> On 25/07/2019 16:12, Jose Abreu wrote:
>>>> From: Jon Hunter <jonathanh@nvidia.com>
>>>> Date: Jul/25/2019, 15:25:59 (UTC+00:00)
>>>>
>>>>>
>>>>> On 25/07/2019 14:26, Jose Abreu wrote:
>>>>>
>>>>> ...
>>>>>
>>>>>> Well, I wasn't expecting that :/
>>>>>>
>>>>>> Per documentation of barriers I think we should set descriptor fields
>>>>>> and then barrier and finally ownership to HW so that remaining fields
>>>>>> are coherent before owner is set.
>>>>>>
>>>>>> Anyway, can you also add a dma_rmb() after the call to
>>>>>> stmmac_rx_status() ?
>>>>>
>>>>> Yes. I removed the debug print added the barrier, but that did not help.
>>>>
>>>> So, I was finally able to setup NFS using your replicated setup and I
>>>> can't see the issue :(
>>>>
>>>> The only difference I have from yours is that I'm using TCP in NFS
>>>> whilst you (I believe from the logs), use UDP.
>>>
>>> So I tried TCP by setting the kernel boot params to 'nfsvers=3' and
>>> 'proto=tcp' and this does appear to be more stable, but not 100% stable.
>>> It still appears to fail in the same place about 50% of the time.
>>>
>>>> You do have flow control active right ? And your HW FIFO size is >= 4k ?
>>>
>>> How can I verify if flow control is active?
>>
>> You can check it by dumping register MTL_RxQ_Operation_Mode (0xd30).
Where would be the appropriate place to dump this? After probe? Maybe
best if you can share a code snippet of where to dump this.
>> Can you also add IOMMU debug in file "drivers/iommu/iommu.c" ?
You can find a boot log here:
https://paste.ubuntu.com/p/qtRqtYKHGF/
> And, please try attached debug patch.
With this patch it appears to boot fine. So far no issues seen.
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH iproute2 0/1] Fix s64 argument parsing
From: Kurt Kanzenbach @ 2019-07-29 11:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20190704122427.22256-1-kurt@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 466 bytes --]
On Thu, Jul 04, 2019 at 02:24:26PM +0200, Kurt Kanzenbach wrote:
> Hi,
>
> while using the TAPRIO Qdisc on ARM32 I've noticed that the base_time parameter is
> incorrectly configured. The problem is the utility function get_s64() used by
> TAPRIO doesn't parse the value correctly.
polite ping.
>
> Thanks,
> Kurt
>
> Kurt Kanzenbach (1):
> utils: Fix get_s64() function
>
> lib/utils.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --
> 2.11.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v4 1/3] flow_offload: move tc indirect block to flow offload
From: Jiri Pirko @ 2019-07-29 11:13 UTC (permalink / raw)
To: wenxu; +Cc: pablo, fw, jakub.kicinski, netfilter-devel, netdev
In-Reply-To: <1564296769-32294-2-git-send-email-wenxu@ucloud.cn>
Sun, Jul 28, 2019 at 08:52:47AM CEST, wenxu@ucloud.cn wrote:
>From: wenxu <wenxu@ucloud.cn>
>
>move tc indirect block to flow_offload and rename
>it to flow indirect block.The nf_tables can use the
>indr block architecture.
>
>Signed-off-by: wenxu <wenxu@ucloud.cn>
>---
>v3: subsys_initcall for init_flow_indr_rhashtable
>v4: no change
>
[...]
>diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>index 00b9aab..66f89bc 100644
>--- a/include/net/flow_offload.h
>+++ b/include/net/flow_offload.h
>@@ -4,6 +4,7 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <net/flow_dissector.h>
>+#include <linux/rhashtable.h>
>
> struct flow_match {
> struct flow_dissector *dissector;
>@@ -366,4 +367,42 @@ static inline void flow_block_init(struct flow_block *flow_block)
> INIT_LIST_HEAD(&flow_block->cb_list);
> }
>
>+typedef int flow_indr_block_bind_cb_t(struct net_device *dev, void *cb_priv,
>+ enum tc_setup_type type, void *type_data);
>+
>+struct flow_indr_block_cb {
>+ struct list_head list;
>+ void *cb_priv;
>+ flow_indr_block_bind_cb_t *cb;
>+ void *cb_ident;
>+};
I don't understand why are you pushing this struct out of the c file to
the header. Please don't.
>+
>+typedef void flow_indr_block_ing_cmd_t(struct net_device *dev,
>+ struct flow_block *flow_block,
>+ struct flow_indr_block_cb *indr_block_cb,
>+ enum flow_block_command command);
>+
>+struct flow_indr_block_dev {
>+ struct rhash_head ht_node;
>+ struct net_device *dev;
>+ unsigned int refcnt;
>+ struct list_head cb_list;
>+ flow_indr_block_ing_cmd_t *ing_cmd_cb;
>+ struct flow_block *flow_block;
I don't understand why are you pushing this struct out of the c file to
the header. Please don't.
>+};
>+
>+struct flow_indr_block_dev *flow_indr_block_dev_lookup(struct net_device *dev);
>+
>+int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+ flow_indr_block_bind_cb_t *cb, void *cb_ident);
>+
>+void __flow_indr_block_cb_unregister(struct net_device *dev,
>+ flow_indr_block_bind_cb_t *cb, void *cb_ident);
>+
>+int flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+ flow_indr_block_bind_cb_t *cb, void *cb_ident);
>+
>+void flow_indr_block_cb_unregister(struct net_device *dev,
>+ flow_indr_block_bind_cb_t *cb, void *cb_ident);
>+
[...]
>+
>+int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+ flow_indr_block_bind_cb_t *cb,
>+ void *cb_ident)
>+{
>+ struct flow_indr_block_cb *indr_block_cb;
>+ struct flow_indr_block_dev *indr_dev;
>+ int err;
>+
>+ indr_dev = flow_indr_block_dev_get(dev);
>+ if (!indr_dev)
>+ return -ENOMEM;
>+
>+ indr_block_cb = flow_indr_block_cb_add(indr_dev, cb_priv, cb, cb_ident);
>+ err = PTR_ERR_OR_ZERO(indr_block_cb);
>+ if (err)
>+ goto err_dev_put;
>+
>+ if (indr_dev->ing_cmd_cb)
>+ indr_dev->ing_cmd_cb(indr_dev->dev, indr_dev->flow_block, indr_block_cb,
This line is over 80cols. Please run checkpatch script for your patch
and obey the warnings.
>+ FLOW_BLOCK_BIND);
>+
>+ return 0;
>+
>+err_dev_put:
>+ flow_indr_block_dev_put(indr_dev);
>+ return err;
>+}
>+EXPORT_SYMBOL_GPL(__flow_indr_block_cb_register);
[...]
>
>-static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
>- struct tc_indr_block_cb *indr_block_cb,
>+static void tc_indr_block_ing_cmd(struct net_device *dev,
I don't understand why you change struct tc_indr_block_dev * to
struct net_device * here. If you want to do that, please do that in a
separate patch, not it this one where only "the move" should happen.
>+ struct flow_block *flow_block,
>+ struct flow_indr_block_cb *indr_block_cb,
> enum flow_block_command command)
> {
>+ struct tcf_block *block = flow_block ?
>+ container_of(flow_block,
>+ struct tcf_block,
>+ flow_block) : NULL;
> struct flow_block_offload bo = {
> .command = command,
> .binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
>- .net = dev_net(indr_dev->dev),
>- .block_shared = tcf_block_non_null_shared(indr_dev->block),
>+ .net = dev_net(dev),
>+ .block_shared = tcf_block_non_null_shared(block),
> };
> INIT_LIST_HEAD(&bo.cb_list);
>
[...]
^ permalink raw reply
* Re: [PATCH net-next v4 1/3] flow_offload: move tc indirect block to flow offload
From: Jiri Pirko @ 2019-07-29 11:15 UTC (permalink / raw)
To: wenxu; +Cc: pablo, fw, jakub.kicinski, netfilter-devel, netdev
In-Reply-To: <1564296769-32294-2-git-send-email-wenxu@ucloud.cn>
Sun, Jul 28, 2019 at 08:52:47AM CEST, wenxu@ucloud.cn wrote:
>From: wenxu <wenxu@ucloud.cn>
>
>move tc indirect block to flow_offload and rename
A sentence should start with capital letter.
>it to flow indirect block.The nf_tables can use the
There should be a space between "." and first letter of the next
sensence.
>indr block architecture.
>
[...]
^ permalink raw reply
* tcan4x5x on a Raspberry Pi
From: FIXED-TERM Buecheler Konstantin (ETAS-SEC/ECT-Mu) @ 2019-07-29 11:19 UTC (permalink / raw)
To: linux-can@vger.kernel.org, netdev@vger.kernel.org; +Cc: dmurphy@ti.com
Hi all,
I am currently working on a project where I am trying to use the tcan4550 chip with a Raspberry PI 3B.
I am struggling to create a working device tree overlay file for the Raspberry Pi.
Has anyone here tried this already? I would appreciate any help.
Thanks,
Konstantin
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-29 11:29 UTC (permalink / raw)
To: Jon Hunter, Jose Abreu, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Catalin Marinas,
Will Deacon
Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai,
Maxime Coquelin, linux-tegra, Giuseppe Cavallaro, Robin Murphy,
David S . Miller
In-Reply-To: <b99b1e49-0cbc-2c66-6325-50fa6f263d91@nvidia.com>
++ Catalin, Will (ARM64 Maintainers)
From: Jon Hunter <jonathanh@nvidia.com>
Date: Jul/29/2019, 11:55:18 (UTC+00:00)
>
> On 29/07/2019 09:16, Jose Abreu wrote:
> > From: Jose Abreu <joabreu@synopsys.com>
> > Date: Jul/27/2019, 16:56:37 (UTC+00:00)
> >
> >> From: Jon Hunter <jonathanh@nvidia.com>
> >> Date: Jul/26/2019, 15:11:00 (UTC+00:00)
> >>
> >>>
> >>> On 25/07/2019 16:12, Jose Abreu wrote:
> >>>> From: Jon Hunter <jonathanh@nvidia.com>
> >>>> Date: Jul/25/2019, 15:25:59 (UTC+00:00)
> >>>>
> >>>>>
> >>>>> On 25/07/2019 14:26, Jose Abreu wrote:
> >>>>>
> >>>>> ...
> >>>>>
> >>>>>> Well, I wasn't expecting that :/
> >>>>>>
> >>>>>> Per documentation of barriers I think we should set descriptor fields
> >>>>>> and then barrier and finally ownership to HW so that remaining fields
> >>>>>> are coherent before owner is set.
> >>>>>>
> >>>>>> Anyway, can you also add a dma_rmb() after the call to
> >>>>>> stmmac_rx_status() ?
> >>>>>
> >>>>> Yes. I removed the debug print added the barrier, but that did not help.
> >>>>
> >>>> So, I was finally able to setup NFS using your replicated setup and I
> >>>> can't see the issue :(
> >>>>
> >>>> The only difference I have from yours is that I'm using TCP in NFS
> >>>> whilst you (I believe from the logs), use UDP.
> >>>
> >>> So I tried TCP by setting the kernel boot params to 'nfsvers=3' and
> >>> 'proto=tcp' and this does appear to be more stable, but not 100% stable.
> >>> It still appears to fail in the same place about 50% of the time.
> >>>
> >>>> You do have flow control active right ? And your HW FIFO size is >= 4k ?
> >>>
> >>> How can I verify if flow control is active?
> >>
> >> You can check it by dumping register MTL_RxQ_Operation_Mode (0xd30).
>
> Where would be the appropriate place to dump this? After probe? Maybe
> best if you can share a code snippet of where to dump this.
>
> >> Can you also add IOMMU debug in file "drivers/iommu/iommu.c" ?
>
> You can find a boot log here:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__paste.ubuntu.com_p_qtRqtYKHGF_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=NrxsR2etpZHGb7HkN4XdgaGmKM1XYyldihNPL6qVSv0&s=CMATEcHVoqZw4sIrNOXc7SFE_kV_5CO5EU21-yJez6c&e=
>
> > And, please try attached debug patch.
>
> With this patch it appears to boot fine. So far no issues seen.
Thank you for testing.
Hi Catalin and Will,
Sorry to add you in such a long thread but we are seeing a DMA issue
with stmmac driver in an ARM64 platform with IOMMU enabled.
The issue seems to be solved when buffers allocation for DMA based
transfers are *not* mapped with the DMA_ATTR_SKIP_CPU_SYNC flag *OR*
when IOMMU is disabled.
Notice that after transfer is done we do use
dma_sync_single_for_{cpu,device} and then we reuse *the same* page for
another transfer.
Can you please comment on whether DMA_ATTR_SKIP_CPU_SYNC can not be used
in ARM64 platforms with IOMMU ?
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* [PATCH] arcnet: com90xx: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-29 11:09 UTC (permalink / raw)
To: Michael Grzeschik, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva, Stephen Rothwell,
Kees Cook
Mark switch cases where we are expecting to fall through.
This patch fixes the following warnings (Building: powerpc allyesconfig):
drivers/net/arcnet/com90xx.c: In function 'com90xx_setup':
include/linux/printk.h:304:2: warning: this statement may fall through [-Wimplicit-fallthrough=]
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/arcnet/com90xx.c:695:3: note: in expansion of macro 'pr_err'
pr_err("Too many arguments\n");
^~~~~~
drivers/net/arcnet/com90xx.c:696:2: note: here
case 3: /* Mem address */
^~~~
drivers/net/arcnet/com90xx.c:697:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
shmem = ints[3];
~~~~~~^~~~~~~~~
drivers/net/arcnet/com90xx.c:698:2: note: here
case 2: /* IRQ */
^~~~
drivers/net/arcnet/com90xx.c:699:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
irq = ints[2];
~~~~^~~~~~~~~
drivers/net/arcnet/com90xx.c:700:2: note: here
case 1: /* IO address */
^~~~
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/arcnet/com90xx.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/arcnet/com90xx.c b/drivers/net/arcnet/com90xx.c
index ca4a57c30bf8..bd75d06ad7df 100644
--- a/drivers/net/arcnet/com90xx.c
+++ b/drivers/net/arcnet/com90xx.c
@@ -693,10 +693,13 @@ static int __init com90xx_setup(char *s)
switch (ints[0]) {
default: /* ERROR */
pr_err("Too many arguments\n");
+ /* Fall through */
case 3: /* Mem address */
shmem = ints[3];
+ /* Fall through */
case 2: /* IRQ */
irq = ints[2];
+ /* Fall through */
case 1: /* IO address */
io = ints[1];
}
--
2.22.0
^ permalink raw reply related
* [PATCH] arcnet: com90io: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-29 11:13 UTC (permalink / raw)
To: Michael Grzeschik, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva, Stephen Rothwell,
Kees Cook
Mark switch cases where we are expecting to fall through.
This patch fixes the following warnings (Building: powerpc allyesconfig):
drivers/net/arcnet/com90io.c: In function 'com90io_setup':
include/linux/printk.h:304:2: warning: this statement may fall through [-Wimplicit-fallthrough=]
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/arcnet/com90io.c:365:3: note: in expansion of macro 'pr_err'
pr_err("Too many arguments\n");
^~~~~~
drivers/net/arcnet/com90io.c:366:2: note: here
case 2: /* IRQ */
^~~~
drivers/net/arcnet/com90io.c:367:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
irq = ints[2];
~~~~^~~~~~~~~
drivers/net/arcnet/com90io.c:368:2: note: here
case 1: /* IO address */
^~~~
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/arcnet/com90io.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/arcnet/com90io.c b/drivers/net/arcnet/com90io.c
index 2c546013a980..186bbf87bc84 100644
--- a/drivers/net/arcnet/com90io.c
+++ b/drivers/net/arcnet/com90io.c
@@ -363,8 +363,10 @@ static int __init com90io_setup(char *s)
switch (ints[0]) {
default: /* ERROR */
pr_err("Too many arguments\n");
+ /* Fall through */
case 2: /* IRQ */
irq = ints[2];
+ /* Fall through */
case 1: /* IO address */
io = ints[1];
}
--
2.22.0
^ permalink raw reply related
* [PATCH] arcnet: arc-rimi: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-29 11:15 UTC (permalink / raw)
To: Michael Grzeschik, David S. Miller
Cc: netdev, linux-kernel, Gustavo A. R. Silva, Stephen Rothwell,
Kees Cook
Mark switch cases where we are expecting to fall through.
This patch fixes the following warnings (Building: powerpc allyesconfig):
drivers/net/arcnet/arc-rimi.c: In function 'arcrimi_setup':
include/linux/printk.h:304:2: warning: this statement may fall through [-Wimplicit-fallthrough=]
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/arcnet/arc-rimi.c:365:3: note: in expansion of macro 'pr_err'
pr_err("Too many arguments\n");
^~~~~~
drivers/net/arcnet/arc-rimi.c:366:2: note: here
case 3: /* Node ID */
^~~~
drivers/net/arcnet/arc-rimi.c:367:8: warning: this statement may fall through [-Wimplicit-fallthrough=]
node = ints[3];
~~~~~^~~~~~~~~
drivers/net/arcnet/arc-rimi.c:368:2: note: here
case 2: /* IRQ */
^~~~
drivers/net/arcnet/arc-rimi.c:369:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
irq = ints[2];
~~~~^~~~~~~~~
drivers/net/arcnet/arc-rimi.c:370:2: note: here
case 1: /* IO address */
^~~~
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/net/arcnet/arc-rimi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/arcnet/arc-rimi.c b/drivers/net/arcnet/arc-rimi.c
index 11c5bad95226..14a5fb378145 100644
--- a/drivers/net/arcnet/arc-rimi.c
+++ b/drivers/net/arcnet/arc-rimi.c
@@ -363,10 +363,13 @@ static int __init arcrimi_setup(char *s)
switch (ints[0]) {
default: /* ERROR */
pr_err("Too many arguments\n");
+ /* Fall through */
case 3: /* Node ID */
node = ints[3];
+ /* Fall through */
case 2: /* IRQ */
irq = ints[2];
+ /* Fall through */
case 1: /* IO address */
io = ints[1];
}
--
2.22.0
^ permalink raw reply related
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Robin Murphy @ 2019-07-29 11:52 UTC (permalink / raw)
To: Jose Abreu, Jon Hunter, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Catalin Marinas,
Will Deacon
Cc: Joao Pinto, Alexandre Torgue, Maxime Ripard, Chen-Yu Tsai,
Maxime Coquelin, linux-tegra, Giuseppe Cavallaro,
David S . Miller
In-Reply-To: <MN2PR12MB327997BDF2EA5CEE00F45AC3D3DD0@MN2PR12MB3279.namprd12.prod.outlook.com>
On 29/07/2019 12:29, Jose Abreu wrote:
> ++ Catalin, Will (ARM64 Maintainers)
>
> From: Jon Hunter <jonathanh@nvidia.com>
> Date: Jul/29/2019, 11:55:18 (UTC+00:00)
>
>>
>> On 29/07/2019 09:16, Jose Abreu wrote:
>>> From: Jose Abreu <joabreu@synopsys.com>
>>> Date: Jul/27/2019, 16:56:37 (UTC+00:00)
>>>
>>>> From: Jon Hunter <jonathanh@nvidia.com>
>>>> Date: Jul/26/2019, 15:11:00 (UTC+00:00)
>>>>
>>>>>
>>>>> On 25/07/2019 16:12, Jose Abreu wrote:
>>>>>> From: Jon Hunter <jonathanh@nvidia.com>
>>>>>> Date: Jul/25/2019, 15:25:59 (UTC+00:00)
>>>>>>
>>>>>>>
>>>>>>> On 25/07/2019 14:26, Jose Abreu wrote:
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>> Well, I wasn't expecting that :/
>>>>>>>>
>>>>>>>> Per documentation of barriers I think we should set descriptor fields
>>>>>>>> and then barrier and finally ownership to HW so that remaining fields
>>>>>>>> are coherent before owner is set.
>>>>>>>>
>>>>>>>> Anyway, can you also add a dma_rmb() after the call to
>>>>>>>> stmmac_rx_status() ?
>>>>>>>
>>>>>>> Yes. I removed the debug print added the barrier, but that did not help.
>>>>>>
>>>>>> So, I was finally able to setup NFS using your replicated setup and I
>>>>>> can't see the issue :(
>>>>>>
>>>>>> The only difference I have from yours is that I'm using TCP in NFS
>>>>>> whilst you (I believe from the logs), use UDP.
>>>>>
>>>>> So I tried TCP by setting the kernel boot params to 'nfsvers=3' and
>>>>> 'proto=tcp' and this does appear to be more stable, but not 100% stable.
>>>>> It still appears to fail in the same place about 50% of the time.
>>>>>
>>>>>> You do have flow control active right ? And your HW FIFO size is >= 4k ?
>>>>>
>>>>> How can I verify if flow control is active?
>>>>
>>>> You can check it by dumping register MTL_RxQ_Operation_Mode (0xd30).
>>
>> Where would be the appropriate place to dump this? After probe? Maybe
>> best if you can share a code snippet of where to dump this.
>>
>>>> Can you also add IOMMU debug in file "drivers/iommu/iommu.c" ?
>>
>> You can find a boot log here:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__paste.ubuntu.com_p_qtRqtYKHGF_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=NrxsR2etpZHGb7HkN4XdgaGmKM1XYyldihNPL6qVSv0&s=CMATEcHVoqZw4sIrNOXc7SFE_kV_5CO5EU21-yJez6c&e=
>>
>>> And, please try attached debug patch.
>>
>> With this patch it appears to boot fine. So far no issues seen.
>
> Thank you for testing.
>
> Hi Catalin and Will,
>
> Sorry to add you in such a long thread but we are seeing a DMA issue
> with stmmac driver in an ARM64 platform with IOMMU enabled.
>
> The issue seems to be solved when buffers allocation for DMA based
> transfers are *not* mapped with the DMA_ATTR_SKIP_CPU_SYNC flag *OR*
> when IOMMU is disabled.
>
> Notice that after transfer is done we do use
> dma_sync_single_for_{cpu,device} and then we reuse *the same* page for
> another transfer.
>
> Can you please comment on whether DMA_ATTR_SKIP_CPU_SYNC can not be used
> in ARM64 platforms with IOMMU ?
In terms of what they do, there should be no difference on arm64 between:
dma_map_page(..., dir);
...
dma_unmap_page(..., dir);
and:
dma_map_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
dma_sync_single_for_device(..., dir);
...
dma_sync_single_for_cpu(..., dir);
dma_unmap_page_attrs(..., dir, DMA_ATTR_SKIP_CPU_SYNC);
provided that the first sync covers the whole buffer and any subsequent
ones cover at least the parts of the buffer which may have changed. Plus
for coherent hardware it's entirely moot either way.
Given Jon's previous findings, I would lean towards the idea that
performing the extra (redundant) cache maintenance plus barrier in
dma_unmap is mostly just perturbing timing in the same way as the debug
print which also made things seem OK.
Robin.
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Allan W. Nielsen @ 2019-07-29 12:14 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <b755f613-e6d8-a2e6-16cd-6f13ec0a6ddc@cumulusnetworks.com>
Hi Nikolay,
First of all, as mentioned further down in this thread, I realized that our
implementation of the multicast floodmasks does not align with the existing SW
implementation. We will change this, such that all multicast packets goes to the
SW bridge.
This changes things a bit, not that much.
I actually think you summarized the issue we have (after changing to multicast
flood-masks) right here:
The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
> >> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
> >> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
> >> Thus only the flooding may need to be controlled.
This seems to be exactly what we need.
Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
on a network where we want to limit the flooding of frames with dmac
01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
One way of doing this could potentially be to support the following command:
bridge fdb add 01:21:6C:00:00:01 port eth0
bridge fdb append 01:21:6C:00:00:01 port eth1
On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
> >>>>>> In general NLM_F_APPEND is only used in vxlan, the bridge does not
> >>>>>> handle that flag at all. FDB is only for *unicast*, nothing is joined
> >>>>>> and no multicast should be used with fdbs. MDB is used for multicast
> >>>>>> handling, but both of these are used for forwarding.
This is true, and this should have been addressed in the patch, we were too
focused on setting up the offload patch in the driver, and forgot to do the SW
implementation.
Do you see any issues in supporting this flag, and updating the SW
forwarding in br_handle_frame_finish such that it can support/allow a FDB entry
to be a multicast?
/Allan
^ permalink raw reply
* [PATCH net-next] MAINTAINERS: Remove mailing-list entry for XDP (eXpress Data Path)
From: Jesper Dangaard Brouer @ 2019-07-29 12:16 UTC (permalink / raw)
Cc: Jesper Dangaard Brouer, xdp-newbies, netdev, bpf, ast, daniel,
davem, jakub.kicinski, john.fastabend
This removes the mailing list xdp-newbies@vger.kernel.org from the XDP
kernel maintainers entry.
Being in the kernel MAINTAINERS file successfully caused the list to
receive kbuild bot warnings, syzbot reports and sometimes developer
patches. The level of details in these messages, doesn't match the
target audience of the XDP-newbies list. This is based on a survey on
the mailing list, where 73% voted for removal from MAINTAINERS file.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9cc156c58f0c..45cb4237eddc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17560,7 +17560,6 @@ M: Jakub Kicinski <jakub.kicinski@netronome.com>
M: Jesper Dangaard Brouer <hawk@kernel.org>
M: John Fastabend <john.fastabend@gmail.com>
L: netdev@vger.kernel.org
-L: xdp-newbies@vger.kernel.org
L: bpf@vger.kernel.org
S: Supported
F: net/core/xdp.c
^ permalink raw reply related
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-29 12:22 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <20190729121409.wa47uelw5f6l4vs4@lx-anielsen.microsemi.net>
Hi Allan,
On 29/07/2019 15:14, Allan W. Nielsen wrote:
> Hi Nikolay,
>
> First of all, as mentioned further down in this thread, I realized that our
> implementation of the multicast floodmasks does not align with the existing SW
> implementation. We will change this, such that all multicast packets goes to the
> SW bridge.
>
> This changes things a bit, not that much.
>
> I actually think you summarized the issue we have (after changing to multicast
> flood-masks) right here:
>
> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
>>>> Thus only the flooding may need to be controlled.
>
> This seems to be exactly what we need.
>
> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
> on a network where we want to limit the flooding of frames with dmac
> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
>
> One way of doing this could potentially be to support the following command:
>
> bridge fdb add 01:21:6C:00:00:01 port eth0
> bridge fdb append 01:21:6C:00:00:01 port eth1
>
> On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
>>>>>>>> In general NLM_F_APPEND is only used in vxlan, the bridge does not
>>>>>>>> handle that flag at all. FDB is only for *unicast*, nothing is joined
>>>>>>>> and no multicast should be used with fdbs. MDB is used for multicast
>>>>>>>> handling, but both of these are used for forwarding.
> This is true, and this should have been addressed in the patch, we were too
> focused on setting up the offload patch in the driver, and forgot to do the SW
> implementation.
>
> Do you see any issues in supporting this flag, and updating the SW
> forwarding in br_handle_frame_finish such that it can support/allow a FDB entry
> to be a multicast?
>
Yes, all of the multicast code is handled differently, it doesn't go through the fdb
lookup or code at all. I don't see how you'll do a lookup in the fdb table with a
multicast mac address, take a look at br_handle_frame_finish() and you'll notice
that when a multicast dmac is detected then we use the bridge mcast code for lookups
and forwarding. If you're trying to achieve Rx only on the bridge of these then
why not just use Ido's tc suggestion or even the ip maddr add offload for each port ?
If you add a multicast mac in the fdb (currently allowed, but has no effect) and you
use dev_mc_add() as suggested that'd just be a hack to pass it down and it is already
possible to achieve via other methods, no need to go through the bridge.
> /Allan
>
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Allan W. Nielsen @ 2019-07-29 12:43 UTC (permalink / raw)
To: Ido Schimmel
Cc: Andrew Lunn, Horatiu Vultur, Nikolay Aleksandrov, roopa, davem,
bridge, netdev, linux-kernel
In-Reply-To: <20190729060923.GA16938@splinter>
Hi Ido,
The 07/29/2019 09:09, Ido Schimmel wrote:
> External E-Mail
>
>
> On Sun, Jul 28, 2019 at 09:15:59PM +0200, Allan W. Nielsen wrote:
> > If we assume that the SwitchDev driver implemented such that all multicast
> > traffic goes to the CPU, then we should really have a way to install a HW
> > offload path in the silicon, such that these packets does not go to the CPU (as
> > they are known not to be use full, and a frame every 3 us is a significant load
> > on small DMA connections and CPU resources).
> >
> > If we assume that the SwitchDev driver implemented such that only "needed"
> > multicast packets goes to the CPU, then we need a way to get these packets in
> > case we want to implement the DLR protocol.
>
> I'm not familiar with the HW you're working with, so the below might not
> be relevant.
>
> In case you don't want to send all multicast traffic to the CPU (I'll
> refer to it later), you can install an ingress tc filter that traps to
> the CPU the packets you do want to receive. Something like:
>
> # tc qdisc add dev swp1 clsact
> # tc filter add dev swp1 pref 1 ingress flower skip_sw dst_mac \
> 01:21:6C:00:00:01 action trap
I have actually been looking at this, and it may an idea to go down this road.
But so far we have chosen not to for the following reasons:
- It is not only about trapping traffic to the CPU, we also needs to capability
to limit the flooding on the front ports.
- In our case (the silicon), this feature really belongs to the MAC-table, which
is why we did prefer to do it via the FDB entries.
- But the HW does have TCAM resources, and we are planning on exposing these
resources via the tc-flower interface. It is just that we have more MAC
table resoruces than TCAM resources, which is another argument for using the
MAC table.
> If your HW supports sharing the same filter among multiple ports, then
> you can install your filter in a tc shared block and bind multiple ports
> to it.
It does, thanks for making us aware of this optimization option.
> Another option is to always send a *copy* of multicast packets to the
> CPU, but make sure the HW uses a policer that prevents the CPU from
> being overwhelmed. To avoid packets being forwarded twice (by HW and
> SW), you will need to mark such packets in your driver with
> 'skb->offload_fwd_mark = 1'.
Understood
> Now, in case user wants to allow the CPU to receive certain packets at a
> higher rate, a tc filter can be used. It will be identical to the filter
> I mentioned earlier, but with a 'police' action chained before 'trap'.
I see.
> I don't think this is currently supported by any driver, but I believe
> it's the right way to go: By default the CPU receives all the traffic it
> should receive and user can fine-tune it using ACLs.
If all the frames goes to the CPU, then how can I fine-tune frames not to go to
the CPU?? I can do a TRAP (to get it to the CPU) a DROP (to drop it before
forwarding), but how can I forward a multicast packet, but prevent it from going
to the CPU?
I have seen that the mirror command can do re-direction, but not to a list of
ports...
All in all, thanks a lot for the suggestions, but to begin with I think we will
explore the MAC table option a bit more. But we will get back to TC to support
the ACL functions.
/Allan
^ permalink raw reply
* Re: [PATCH net-next v4 1/3] flow_offload: move tc indirect block to flow offload
From: wenxu @ 2019-07-29 12:47 UTC (permalink / raw)
To: Jiri Pirko; +Cc: pablo, fw, jakub.kicinski, netfilter-devel, netdev
In-Reply-To: <20190729111350.GE2211@nanopsycho>
在 2019/7/29 19:13, Jiri Pirko 写道:
> Sun, Jul 28, 2019 at 08:52:47AM CEST, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> move tc indirect block to flow_offload and rename
>> it to flow indirect block.The nf_tables can use the
>> indr block architecture.
>>
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> ---
>> v3: subsys_initcall for init_flow_indr_rhashtable
>> v4: no change
>>
> [...]
>
>
>> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>> index 00b9aab..66f89bc 100644
>> --- a/include/net/flow_offload.h
>> +++ b/include/net/flow_offload.h
>> @@ -4,6 +4,7 @@
>> #include <linux/kernel.h>
>> #include <linux/list.h>
>> #include <net/flow_dissector.h>
>> +#include <linux/rhashtable.h>
>>
>> struct flow_match {
>> struct flow_dissector *dissector;
>> @@ -366,4 +367,42 @@ static inline void flow_block_init(struct flow_block *flow_block)
>> INIT_LIST_HEAD(&flow_block->cb_list);
>> }
>>
>> +typedef int flow_indr_block_bind_cb_t(struct net_device *dev, void *cb_priv,
>> + enum tc_setup_type type, void *type_data);
>> +
>> +struct flow_indr_block_cb {
>> + struct list_head list;
>> + void *cb_priv;
>> + flow_indr_block_bind_cb_t *cb;
>> + void *cb_ident;
>> +};
> I don't understand why are you pushing this struct out of the c file to
> the header. Please don't.
>
>
>> +
>> +typedef void flow_indr_block_ing_cmd_t(struct net_device *dev,
>> + struct flow_block *flow_block,
>> + struct flow_indr_block_cb *indr_block_cb,
>> + enum flow_block_command command);
>> +
>> +struct flow_indr_block_dev {
>> + struct rhash_head ht_node;
>> + struct net_device *dev;
>> + unsigned int refcnt;
>> + struct list_head cb_list;
>> + flow_indr_block_ing_cmd_t *ing_cmd_cb;
>> + struct flow_block *flow_block;
> I don't understand why are you pushing this struct out of the c file to
> the header. Please don't.
the flow_indr_block_dev and indr_block_cb in the h file used for the function
tc_indr_block_ing_cmd in cls_api.c
>> -static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
>> - struct tc_indr_block_cb *indr_block_cb,
>> +static void tc_indr_block_ing_cmd(struct net_device *dev,
> I don't understand why you change struct tc_indr_block_dev * to
> struct net_device * here. If you want to do that, please do that in a
> separate patch, not it this one where only "the move" should happen.
>
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Nikolay Aleksandrov @ 2019-07-29 12:50 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Horatiu Vultur, roopa, davem, bridge, netdev, linux-kernel
In-Reply-To: <95315f9e-0d31-2d34-ba50-11e1bbc1465c@cumulusnetworks.com>
On 29/07/2019 15:22, Nikolay Aleksandrov wrote:
> Hi Allan,
> On 29/07/2019 15:14, Allan W. Nielsen wrote:
>> Hi Nikolay,
>>
>> First of all, as mentioned further down in this thread, I realized that our
>> implementation of the multicast floodmasks does not align with the existing SW
>> implementation. We will change this, such that all multicast packets goes to the
>> SW bridge.
>>
>> This changes things a bit, not that much.
>>
>> I actually think you summarized the issue we have (after changing to multicast
>> flood-masks) right here:
>>
>> The 07/26/2019 12:26, Nikolay Aleksandrov wrote:
>>>>> Actually you mentioned non-IP traffic, so the querier stuff is not a problem. This
>>>>> traffic will always be flooded by the bridge (and also a copy will be locally sent up).
>>>>> Thus only the flooding may need to be controlled.
>>
>> This seems to be exactly what we need.
>>
>> Assuming we have a SW bridge (br0) with 4 slave interfaces (eth0-3). We use this
>> on a network where we want to limit the flooding of frames with dmac
>> 01:21:6C:00:00:01 (which is non IP traffic) to eth0 and eth1.
>>
>> One way of doing this could potentially be to support the following command:
>>
>> bridge fdb add 01:21:6C:00:00:01 port eth0
>> bridge fdb append 01:21:6C:00:00:01 port eth1
>>
And the fdbs become linked lists ? So we'll increase the complexity for something
that is already supported by ACLs (e.g. tc) and also bridge per-port multicast
flood flag ?
I'm sorry but that doesn't sound good to me for a case which is very rare and
there are existing ways to solve without incurring performance hits or increasing
code complexity.
>> On 25/07/2019 16:06, Nikolay Aleksandrov wrote:
>>>>>>>>> In general NLM_F_APPEND is only used in vxlan, the bridge does not
>>>>>>>>> handle that flag at all. FDB is only for *unicast*, nothing is joined
>>>>>>>>> and no multicast should be used with fdbs. MDB is used for multicast
>>>>>>>>> handling, but both of these are used for forwarding.
>> This is true, and this should have been addressed in the patch, we were too
>> focused on setting up the offload patch in the driver, and forgot to do the SW
>> implementation.
>>
>> Do you see any issues in supporting this flag, and updating the SW
>> forwarding in br_handle_frame_finish such that it can support/allow a FDB entry
>> to be a multicast?
>>
>
> Yes, all of the multicast code is handled differently, it doesn't go through the fdb
> lookup or code at all. I don't see how you'll do a lookup in the fdb table with a
> multicast mac address, take a look at br_handle_frame_finish() and you'll notice
> that when a multicast dmac is detected then we use the bridge mcast code for lookups
> and forwarding. If you're trying to achieve Rx only on the bridge of these then
> why not just use Ido's tc suggestion or even the ip maddr add offload for each port ?
>
> If you add a multicast mac in the fdb (currently allowed, but has no effect) and you
> use dev_mc_add() as suggested that'd just be a hack to pass it down and it is already
> possible to achieve via other methods, no need to go through the bridge.
>
>> /Allan
>>
>
^ permalink raw reply
* RE: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions
From: Michal Kalderon @ 2019-07-29 12:58 UTC (permalink / raw)
To: Jason Gunthorpe, galpress@amazon.com
Cc: Ariel Elior, dledford@redhat.com, galpress@amazon.com,
linux-rdma@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <20190725175540.GA18757@ziepe.ca>
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
>
> > + xa_lock(&ucontext->mmap_xa);
> > + if (check_add_overflow(ucontext->mmap_xa_page,
> > + (u32)(length >> PAGE_SHIFT),
> > + &next_mmap_page))
> > + goto err_unlock;
>
> I still don't like that this algorithm latches into a permanent failure when the
> xa_page wraps.
>
> It seems worth spending a bit more time here to tidy this.. Keep using the
> mmap_xa_page scheme, but instead do something like
>
> alloc_cyclic_range():
>
> while () {
> // Find first empty element in a cyclic way
> xa_page_first = mmap_xa_page;
> xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
>
> // Is there a enough room to have the range?
> if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
> mmap_xa_page = 0;
> continue;
> }
>
> // See if the element before intersects
> elm = xa_find(xa, &zero, xa_page_end, 0);
> if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm->last)) {
> mmap_xa_page = elm->last + 1;
> continue
> }
>
> // xa_page_first -> xa_page_end should now be free
> xa_insert(xa, xa_page_start, entry);
> mmap_xa_page = xa_page_end + 1;
> return xa_page_start;
> }
>
> Approximately, please check it.
Gal & Jason,
Coming back to the mmap_xa_page algorithm. I couldn't find some background on this.
Why do you need the length to be represented in the mmap_xa_page ?
Why not simply use xa_alloc_cyclic ( like in siw )
This is simply a key to a mmap object...
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH] net: stmmac: manage errors returned by of_get_mac_address()
From: Neil Armstrong @ 2019-07-29 13:03 UTC (permalink / raw)
To: Martin Blumenstingl, peppe.cavallaro, alexandre.torgue, joabreu,
davem, netdev
Cc: linux-amlogic, linux-kernel, linux-arm-kernel
In-Reply-To: <20190727192137.27881-1-martin.blumenstingl@googlemail.com>
On 27/07/2019 21:21, Martin Blumenstingl wrote:
> Commit d01f449c008a ("of_net: add NVMEM support to of_get_mac_address")
> added support for reading the MAC address from an nvmem-cell. This
> required changing the logic to return an error pointer upon failure.
>
> If stmmac is loaded before the nvmem provider driver then
> of_get_mac_address() return an error pointer with -EPROBE_DEFER.
>
> Propagate this error so the stmmac driver will be probed again after the
> nvmem provider driver is loaded.
> Default to a random generated MAC address in case of any other error,
> instead of using the error pointer as MAC address.
>
> Fixes: d01f449c008a ("of_net: add NVMEM support to of_get_mac_address")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 73fc2524372e..154daf4d1072 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -370,6 +370,13 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> return ERR_PTR(-ENOMEM);
>
> *mac = of_get_mac_address(np);
> + if (IS_ERR(*mac)) {
> + if (PTR_ERR(*mac) == -EPROBE_DEFER)
> + return ERR_CAST(*mac);
> +
> + *mac = NULL;
> + }
> +
> plat->interface = of_get_phy_mode(np);
>
> /* Some wrapper drivers still rely on phy_node. Let's save it while
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* [PATCH ethtool] gitignore: ignore vim swapfiles and patches
From: Michal Kubecek @ 2019-07-29 13:10 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev
The .*.swp files are created by vim to hold the undo/redo log. Add them to
.gitignore to prevent "git status" or "git gui" from showing them whenever
some file is open in editor.
Add also *.patch to hide patches created by e.g. "git format-patch".
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
.gitignore | 3 +++
1 file changed, 3 insertions(+)
diff --git a/.gitignore b/.gitignore
index f1165a2c9037..c4df588c37ea 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,3 +27,6 @@ autom4te.cache
.deps
test-*.log
test-*.trs
+
+.*.swp
+*.patch
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] tcp: add new tcp_mtu_probe_floor sysctl
From: Neal Cardwell @ 2019-07-29 13:12 UTC (permalink / raw)
To: Josh Hunt; +Cc: Eric Dumazet, netdev, David Miller
In-Reply-To: <5a054ca5-4077-5e91-69d5-f1add8dc8bfa@akamai.com>
On Sun, Jul 28, 2019 at 5:14 PM Josh Hunt <johunt@akamai.com> wrote:
>
> On 7/28/19 6:54 AM, Eric Dumazet wrote:
> > On Sun, Jul 28, 2019 at 1:21 AM Josh Hunt <johunt@akamai.com> wrote:
> >>
> >> On 7/27/19 12:05 AM, Eric Dumazet wrote:
> >>> On Sat, Jul 27, 2019 at 4:23 AM Josh Hunt <johunt@akamai.com> wrote:
> >>>>
> >>>> The current implementation of TCP MTU probing can considerably
> >>>> underestimate the MTU on lossy connections allowing the MSS to get down to
> >>>> 48. We have found that in almost all of these cases on our networks these
> >>>> paths can handle much larger MTUs meaning the connections are being
> >>>> artificially limited. Even though TCP MTU probing can raise the MSS back up
> >>>> we have seen this not to be the case causing connections to be "stuck" with
> >>>> an MSS of 48 when heavy loss is present.
> >>>>
> >>>> Prior to pushing out this change we could not keep TCP MTU probing enabled
> >>>> b/c of the above reasons. Now with a reasonble floor set we've had it
> >>>> enabled for the past 6 months.
> >>>
> >>> And what reasonable value have you used ???
> >>
> >> Reasonable for some may not be reasonable for others hence the new
> >> sysctl :) We're currently running with a fairly high value based off of
> >> the v6 min MTU minus headers and options, etc. We went conservative with
> >> our setting initially as it seemed a reasonable first step when
> >> re-enabling TCP MTU probing since with no configurable floor we saw a #
> >> of cases where connections were using severely reduced mss b/c of loss
> >> and not b/c of actual path restriction. I plan to reevaluate the setting
> >> at some point, but since the probing method is still the same it means
> >> the same clients who got stuck with mss of 48 before will land at
> >> whatever floor we set. Looking forward we are interested in trying to
> >> improve TCP MTU probing so it does not penalize clients like this.
> >>
> >> A suggestion for a more reasonable floor default would be 512, which is
> >> the same as the min_pmtu. Given both mechanisms are trying to achieve
> >> the same goal it seems like they should have a similar min/floor.
> >>
> >>>
> >>>>
> >>>> The new sysctl will still default to TCP_MIN_SND_MSS (48), but gives
> >>>> administrators the ability to control the floor of MSS probing.
> >>>>
> >>>> Signed-off-by: Josh Hunt <johunt@akamai.com>
> >>>> ---
> >>>> Documentation/networking/ip-sysctl.txt | 6 ++++++
> >>>> include/net/netns/ipv4.h | 1 +
> >>>> net/ipv4/sysctl_net_ipv4.c | 9 +++++++++
> >>>> net/ipv4/tcp_ipv4.c | 1 +
> >>>> net/ipv4/tcp_timer.c | 2 +-
> >>>> 5 files changed, 18 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> >>>> index df33674799b5..49e95f438ed7 100644
> >>>> --- a/Documentation/networking/ip-sysctl.txt
> >>>> +++ b/Documentation/networking/ip-sysctl.txt
> >>>> @@ -256,6 +256,12 @@ tcp_base_mss - INTEGER
> >>>> Path MTU discovery (MTU probing). If MTU probing is enabled,
> >>>> this is the initial MSS used by the connection.
> >>>>
> >>>> +tcp_mtu_probe_floor - INTEGER
> >>>> + If MTU probing is enabled this caps the minimum MSS used for search_low
> >>>> + for the connection.
> >>>> +
> >>>> + Default : 48
> >>>> +
> >>>> tcp_min_snd_mss - INTEGER
> >>>> TCP SYN and SYNACK messages usually advertise an ADVMSS option,
> >>>> as described in RFC 1122 and RFC 6691.
> >>>> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
> >>>> index bc24a8ec1ce5..c0c0791b1912 100644
> >>>> --- a/include/net/netns/ipv4.h
> >>>> +++ b/include/net/netns/ipv4.h
> >>>> @@ -116,6 +116,7 @@ struct netns_ipv4 {
> >>>> int sysctl_tcp_l3mdev_accept;
> >>>> #endif
> >>>> int sysctl_tcp_mtu_probing;
> >>>> + int sysctl_tcp_mtu_probe_floor;
> >>>> int sysctl_tcp_base_mss;
> >>>> int sysctl_tcp_min_snd_mss;
> >>>> int sysctl_tcp_probe_threshold;
> >>>> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> >>>> index 0b980e841927..59ded25acd04 100644
> >>>> --- a/net/ipv4/sysctl_net_ipv4.c
> >>>> +++ b/net/ipv4/sysctl_net_ipv4.c
> >>>> @@ -820,6 +820,15 @@ static struct ctl_table ipv4_net_table[] = {
> >>>> .extra2 = &tcp_min_snd_mss_max,
> >>>> },
> >>>> {
> >>>> + .procname = "tcp_mtu_probe_floor",
> >>>> + .data = &init_net.ipv4.sysctl_tcp_mtu_probe_floor,
> >>>> + .maxlen = sizeof(int),
> >>>> + .mode = 0644,
> >>>> + .proc_handler = proc_dointvec_minmax,
> >>>> + .extra1 = &tcp_min_snd_mss_min,
> >>>> + .extra2 = &tcp_min_snd_mss_max,
> >>>> + },
> >>>> + {
> >>>> .procname = "tcp_probe_threshold",
> >>>> .data = &init_net.ipv4.sysctl_tcp_probe_threshold,
> >>>> .maxlen = sizeof(int),
> >>>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> >>>> index d57641cb3477..e0a372676329 100644
> >>>> --- a/net/ipv4/tcp_ipv4.c
> >>>> +++ b/net/ipv4/tcp_ipv4.c
> >>>> @@ -2637,6 +2637,7 @@ static int __net_init tcp_sk_init(struct net *net)
> >>>> net->ipv4.sysctl_tcp_min_snd_mss = TCP_MIN_SND_MSS;
> >>>> net->ipv4.sysctl_tcp_probe_threshold = TCP_PROBE_THRESHOLD;
> >>>> net->ipv4.sysctl_tcp_probe_interval = TCP_PROBE_INTERVAL;
> >>>> + net->ipv4.sysctl_tcp_mtu_probe_floor = TCP_MIN_SND_MSS;
> >>>>
> >>>> net->ipv4.sysctl_tcp_keepalive_time = TCP_KEEPALIVE_TIME;
> >>>> net->ipv4.sysctl_tcp_keepalive_probes = TCP_KEEPALIVE_PROBES;
> >>>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> >>>> index c801cd37cc2a..dbd9d2d0ee63 100644
> >>>> --- a/net/ipv4/tcp_timer.c
> >>>> +++ b/net/ipv4/tcp_timer.c
> >>>> @@ -154,7 +154,7 @@ static void tcp_mtu_probing(struct inet_connection_sock *icsk, struct sock *sk)
> >>>> } else {
> >>>> mss = tcp_mtu_to_mss(sk, icsk->icsk_mtup.search_low) >> 1;
> >>>> mss = min(net->ipv4.sysctl_tcp_base_mss, mss);
> >>>> - mss = max(mss, 68 - tcp_sk(sk)->tcp_header_len);
> >>>> + mss = max(mss, net->ipv4.sysctl_tcp_mtu_probe_floor);
> >>>> mss = max(mss, net->ipv4.sysctl_tcp_min_snd_mss);
> >>>> icsk->icsk_mtup.search_low = tcp_mss_to_mtu(sk, mss);
> >>>> }
> >>>
> >>>
> >>> Existing sysctl should be enough ?
> >>
> >> I don't think so. Changing tcp_min_snd_mss could impact clients that
> >> really want/need a small mss. When you added the new sysctl I tried to
> >> analyze the mss values we're seeing to understand what we could possibly
> >> raise it to. While not a huge amount, we see more clients than I
> >> expected announcing mss values in the 180-512 range. Given that I would
> >> not feel comfortable setting tcp_min_snd_mss to say 512 as I suggested
> >> above.
> >
> > If these clients need mss values in 180-512 ranges, how MTU probing
> > would work for them,
> > if you set a floor to 512 ?
>
> First, we already seem to be fine with ignoring these paths with ICMP
> based PMTU discovery b/c of our min_pmtu default of 512 and that is
> configurable. Second by adding this sysctl we're giving administrators
> the choice to decide if they'd like to attempt to support these very
> very small # of paths which may be below 512 (MSS <= 512 does not mean
> MTU <= 512) or cover themselves by being able to raise the floor to not
> penalize clients who may be on very lossy networks.
>
> >
> > Are we sure the intent of tcp_base_mss was not to act as a floor ?
>
> My understanding is that tcp_base_mss is meant to be the initial value
> of search_low (as per Docs). Then in RFC 4821 [1] Sections 7.2, shows
> search_low should be configurable, and 7.7 we see that in response to
> successive black hole detection search_low should be halved. So I don't
> think it was meant to be a floor, but just the initial search_low param.
That matches my reading of the RFC and code as well. But in that case
IMHO an additional commit should fix this comment to reflect the fact
thatTCP_BASE_MSS is the initial value, rather than a floor:
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 42728239cdbe..05575ac70333 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -75,7 +75,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo);
/* Minimal accepted MSS. It is (60+60+8) - (20+20). */
#define TCP_MIN_MSS 88U
-/* The least MTU to use for probing */
+/* The initial MTU to use for probing */
#define TCP_BASE_MSS 1024
/* probing interval, default to 10 minutes as per RFC4821 */
neal
^ permalink raw reply related
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