* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Eric Dumazet @ 2017-05-09 16:56 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Andrey Konovalov,
Eric Dumazet
In-Reply-To: <CAM_iQpWQoLppsMfDOQWsdifGd56CAY24tCNm0Jm02QCe7ZThOw@mail.gmail.com>
On Tue, 2017-05-09 at 09:44 -0700, Cong Wang wrote:
>
> Eric, how did you produce it?
> I guess it's because of nh_dev which is the only netdevice pointer inside
> fib_info. Let me take a deeper look.
>
Nothing particular, I am using kexec to boot new kernels, and all my
attempts with your patch included demonstrated the issue.
eth0 is a bonding device, it might matter, I do not know.
We also have some tunnels, but unfortunately I can not provide a setup
that you could use on say a VM.
I can send you the .config if this can help
> >>
> >> I am assuming you are quite confident it is this change?
> >
> > At least, reverting the patch resolves the issue for me.
> >
> > Keeping fib (and their reference to netdev) is apparently too much,
> > we probably need to implement a refcount on the metrics themselves,
> > being stand alone objects.
>
> I don't disagree, just that it may need to change too much code which
> goes beyond a stable candidate.
Well, your choice, but dealing with a full blown fib and its
dependencies look fragile to me.
^ permalink raw reply
* [PATCH net-next] net: stmmac: use correct pointer when printing normal descriptor ring
From: Niklas Cassel @ 2017-05-09 16:52 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue; +Cc: Niklas Cassel, netdev, linux-kernel
From: Niklas Cassel <niklas.cassel@axis.com>
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cd8c60132390..a74c481401c4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3725,7 +3725,7 @@ static void sysfs_display_ring(void *head, int size, int extend_desc,
ep++;
} else {
seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
- i, (unsigned int)virt_to_phys(ep),
+ i, (unsigned int)virt_to_phys(p),
le32_to_cpu(p->des0), le32_to_cpu(p->des1),
le32_to_cpu(p->des2), le32_to_cpu(p->des3));
p++;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net v2] driver: vrf: Fix one possible use-after-free issue
From: David Ahern @ 2017-05-09 16:51 UTC (permalink / raw)
To: gfree.wind, shm, davem, fw, netdev
In-Reply-To: <1494325653-39885-1-git-send-email-gfree.wind@vip.163.com>
On 5/9/17 3:27 AM, gfree.wind@vip.163.com wrote:
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index ceda586..db88249 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -989,6 +989,7 @@ static u32 vrf_fib_table(const struct net_device *dev)
>
> static int vrf_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
> {
> + kfree_skb(skb);
> return 0;
> }
>
> @@ -998,7 +999,7 @@ static struct sk_buff *vrf_rcv_nfhook(u8 pf, unsigned int hook,
> {
> struct net *net = dev_net(dev);
>
> - if (NF_HOOK(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) < 0)
> + if (nf_hook(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) != 1)
> skb = NULL; /* kfree_skb(skb) handled by nf code */
>
> return skb;
>
I'm clearly misunderstanding something ...
With the current code:
- nf_hook returns 1, NF_HOOK invokes vrf_rcv_finish as the okfn, it
returns 0, skb passes on.
- nf_hook returns 0, vrf_rcv_finish has been called by the nf_hook tree,
vrf_rcv_finish returns 0, skb passes on
- nf_hook returns < 0, vrf_rcv_finish is not called, skb is freed by
netfilter code, vrf_rcv_nfhook returns NULL
What am I missing?
With the above, if nf_hook returns 1, vrf_rcv_finish is not called.
^ permalink raw reply
* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Cong Wang @ 2017-05-09 16:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Linux Kernel Network Developers, Andrey Konovalov,
Eric Dumazet
In-Reply-To: <1494296302.7796.61.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, May 8, 2017 at 7:18 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2017-05-08 at 21:22 -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Mon, 08 May 2017 17:01:20 -0700
>>
>> > On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
>> >> From: Cong Wang <xiyou.wangcong@gmail.com>
>> >> Date: Thu, 4 May 2017 14:54:17 -0700
>> >>
>> >> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
>> >> > itself is refcnt'ed, so without taking a refcnt fi and
>> >> > fi->fib_metrics could be freed while dst metrics still points to
>> >> > it. This triggers use-after-free as reported by Andrey twice.
>> >> >
>> >> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
>> >> > restore this reference counting. It is a quick fix for -net and
>> >> > -stable, for -net-next, as Eric suggested, we can consider doing
>> >> > reference counting for metrics itself instead of relying on fib_info.
>> >> >
>> >> > IPv6 is very different, it copies or steals the metrics from mx6_config
>> >> > in fib6_commit_metrics() so probably doesn't need a refcnt.
>> >> >
>> >> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
>> >> >
>> >> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
>> >> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
>> >> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
>> >> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> >>
>> >> Applied and queued up for -stable, thanks.
>> >
>> > Although I now have on latest net tree these messages when I reboot my
>> > test machine.
>> >
>> > [ 224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
>>
>> Strange, the refcounting looks quite OK in the patch you're quoting.
>> I looked over it a few times and cannot figure out a possible cause
>> there.
Eric, how did you produce it?
I guess it's because of nh_dev which is the only netdevice pointer inside
fib_info. Let me take a deeper look.
>>
>> I am assuming you are quite confident it is this change?
>
> At least, reverting the patch resolves the issue for me.
>
> Keeping fib (and their reference to netdev) is apparently too much,
> we probably need to implement a refcount on the metrics themselves,
> being stand alone objects.
I don't disagree, just that it may need to change too much code which
goes beyond a stable candidate.
Thanks for the bug report!
^ permalink raw reply
* mlx5 endpoint driver problem
From: Joao Pinto @ 2017-05-09 16:25 UTC (permalink / raw)
To: saeedm; +Cc: netdev
Hello,
I am making tests with a Mellanox MLX5 Endpoint, and I am getting kernel hangs
when trying to enable the hca:
mlx5_core 0000:01:00.0: enabling device (0000 -> 0002)
mlx5_core 0000:01:00.0: Warning: couldn't set 64-bit PCI DMA mask
mlx5_core 0000:01:00.0: Warning: couldn't set 64-bit consistent PCI DMA mask
mlx5_core 0000:01:00.0: firmware version: 16.19.21102
INFO: task swapper:1 blocked for more than 10 seconds.
Not tainted 4.11.0-BETAMSIX1 #51
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
swapper D 0 1 0 0x00000000
Stack Trace:
__switch_to+0x0/0x94
__schedule+0x1da/0x8b0
schedule+0x26/0x6c
schedule_timeout+0x2da/0x380
wait_for_completion+0x92/0x104
mlx5_cmd_exec+0x70e/0xd60
mlx5_load_one+0x1b4/0xad8
init_one+0x404/0x600
pci_device_probe+0x122/0x1f0
really_probe+0x1ac/0x348
__driver_attach+0xa8/0xd0
bus_for_each_dev+0x3c/0x74
bus_add_driver+0xc2/0x184
driver_register+0x50/0xec
init+0x40/0x60
(...)
Stack Trace:
__switch_to+0x0/0x94
__schedule+0x1da/0x8b0
schedule+0x26/0x6c
schedule_timeout+0x2da/0x380
wait_for_completion+0x92/0x104
mlx5_cmd_exec+0x70e/0xd60
mlx5_load_one+0x1b4/0xad8
init_one+0x404/0x600
pci_device_probe+0x122/0x1f0
really_probe+0x1ac/0x348
__driver_attach+0xa8/0xd0
bus_for_each_dev+0x3c/0x74
bus_add_driver+0xc2/0x184
driver_register+0x50/0xec
init+0x40/0x60
mlx5_core 0000:01:00.0: wait_func:882:(pid 1): ENABLE_HCA(0x104) timeout. Will
cause a leak of a command resource
mlx5_core 0000:01:00.0: enable hca failed
mlx5_core 0000:01:00.0: mlx5_load_one failed with error code -110
mlx5_core: probe of 0000:01:00.0 failed with error -110
Could you give me a clue of what might be happennig?
Thanks,
Joao
^ permalink raw reply
* [PATCH] netxen_nic: set rcode to the return status from the call to netxen_issue_cmd
From: Colin King @ 2017-05-09 16:19 UTC (permalink / raw)
To: Manish Chopra, Rahul Verma, Dept-GELinuxNICDev, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently rcode is being initialized to NX_RCODE_SUCCESS and later it
is checked to see if it is not NX_RCODE_SUCCESS which is never true. It
appears that there is an unintentional missing assignment of rcode from
the return of the call to netxen_issue_cmd() that was dropped in
an earlier fix, so add it in.
Detected by CoverityScan, CID#401900 ("Logically dead code")
Fixes: 2dcd5d95ad6b2 ("netxen_nic: fix cdrp race condition")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
index b8d5270359cd..e30676515529 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
@@ -247,7 +247,7 @@ nx_fw_cmd_set_mtu(struct netxen_adapter *adapter, int mtu)
cmd.req.arg3 = 0;
if (recv_ctx->state == NX_HOST_CTX_STATE_ACTIVE)
- netxen_issue_cmd(adapter, &cmd);
+ rcode = netxen_issue_cmd(adapter, &cmd);
if (rcode != NX_RCODE_SUCCESS)
return -EIO;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 0/2] net: Set maximum receive packet size on veth interfaces
From: Stephen Hemminger @ 2017-05-09 15:49 UTC (permalink / raw)
To: Fredrik Markstrom
Cc: Eric Dumazet, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, netdev, linux-kernel, bridge
In-Reply-To: <20170509124439.45674-1-fredrik.markstrom@gmail.com>
On Tue, 9 May 2017 14:44:36 +0200
Fredrik Markstrom <fredrik.markstrom@gmail.com> wrote:
> Currently veth drops all packets larger then the mtu set on the receiving
> end of the pair. This is inconsistent with most hardware ethernet drivers.
There is no guarantee that packets larger than MTU + VLAN tag will be received
by hardware drivers. So why is this necessary for veth? What is your special
use case which makes this necessary?
^ permalink raw reply
* openvswitch MTU patch needed in 4.10 stable
From: Stephen Hemminger @ 2017-05-09 15:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Could you queue the patch to stable?
Begin forwarded message:
Date: Tue, 09 May 2017 08:21:46 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 195695] New: openvswitch: Set internal device max mtu to ETH_MAX_MTU
https://bugzilla.kernel.org/show_bug.cgi?id=195695
Bug ID: 195695
Summary: openvswitch: Set internal device max mtu to
ETH_MAX_MTU
Product: Networking
Version: 2.5
Kernel Version: 4.10
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: stephen@networkplumber.org
Reporter: endre@basis-consulting.com
Regression: No
Can not use jumbo frames with openvswitch bridge in kernel 4.10. This is fixed
in kernel 4.11:
Commit 91572088e3fd ("net: use core MTU range checking in core net
infra") changed the openvswitch internal device to use the core net
infra for controlling the MTU range, but failed to actually set the
max_mtu as described in the commit message, which now defaults to
ETH_DATA_LEN.
This patch fixes this by setting max_mtu to ETH_MAX_MTU after
ether_setup() call.
Fixes: 91572088e3fd ("net: use core MTU range checking in core net infra")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/openvswitch/vport-internal_dev.c')
-rw-r--r--
net/openvswitch/vport-internal_dev.c
2
1 files changed, 2 insertions, 0 deletions
diff --git a/net/openvswitch/vport-internal_dev.c
b/net/openvswitch/vport-internal_dev.c
index 09141a1..89193a6 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -149,6 +149,8 @@ static void do_setup(struct net_device *netdev)
{
ether_setup(netdev);
+ netdev->max_mtu = ETH_MAX_MTU;
+
netdev->netdev_ops = &internal_dev_netdev_ops;
netdev->priv_flags &= ~IFF_TX_SKB_SHARING;
Br.
Endre Vaade
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply related
* Re: [PATCH RFC net-next 0/6] net: reducing memory footprint of network devices
From: David Ahern @ 2017-05-09 15:42 UTC (permalink / raw)
To: nicolas.dichtel, Florian Fainelli, netdev; +Cc: roopa
In-Reply-To: <1be60a5e-da39-5657-b1fe-c91266800046@6wind.com>
On 5/9/17 2:50 AM, Nicolas Dichtel wrote:
> Your initial patch tried to make those interfaces transparent, this is not the
> case anymore here. It would probably be useful to be able to filter those
> interfaces in the kernel during a dump.
The earlier email was for hidden devices; the intent there is to hide
certain devices (e.g., switch control netdevs) from user dumps by default.
Adding an attribute at create time such as IFF_INVISIBLE for such
devices would be a follow on to this set - but leveraging the same
sysctl and sysfs bypasses.
^ permalink raw reply
* Re: [PATCH net 0/5] qed*: General fixes
From: David Miller @ 2017-05-09 15:25 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev
In-Reply-To: <1494331671-16273-1-git-send-email-Yuval.Mintz@cavium.com>
From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Tue, 9 May 2017 15:07:46 +0300
> This series contain several fixes for qed and qede.
>
> - #1 [and ~#5] relate to XDP cleanups
> - #2 and #5 correct VF behavior
> - #3 and #4 fix and add missing configurations needed for RoCE & storage
>
> Dave,
>
> Please consider applying the series to 'net'.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net 0/3] mlx4 misc fixes
From: David Miller @ 2017-05-09 15:23 UTC (permalink / raw)
To: tariqt; +Cc: netdev, eranbe
In-Reply-To: <1494330324-11752-1-git-send-email-tariqt@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 9 May 2017 14:45:21 +0300
> This patchset contains misc bug fixes from the team
> to the mlx4 Core and Eth drivers.
>
> Series generated against net commit:
> 32f1bc0f3d26 Revert "ipv4: restore rt->fi for reference counting"
Series applied, thanks Tariq.
^ permalink raw reply
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Joe Perches @ 2017-05-09 15:18 UTC (permalink / raw)
To: Florian Fainelli, Julia Lawall
Cc: David Laight, 'Christophe JAILLET', andrew@lunn.ch,
vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <f9058de7-ef9e-f6eb-751d-72ffdce512bb@gmail.com>
On Mon, 2017-05-08 at 17:35 -0700, Florian Fainelli wrote:
> On 05/08/2017 04:46 PM, Julia Lawall wrote:
> > On Mon, 8 May 2017, Joe Perches wrote:
> > > Each time -EPROBE_DEFER occurs, another set of calls to
> > > dsa_switch_alloc and dev_kzalloc also occurs.
> > >
> > > Perhaps it'd be better to do:
> > >
> > > if (ps->netdev) {
> > > devm_kfree(&devmdev->dev, ps);
> > > devm_kfree(&mdiodev->dev, ds);
> > > return -EPROBE_DEFER;
> > > }
> >
> > Is EPROBE_DEFER handled differently than other kinds of errors?
>
> In the core device driver model, yes, EPROBE_DEFER is treated
> differently than other errors because it puts the driver on a retry queue.
>
> EPROBE_DEFER is already a slow and exceptional path, and this is a
> mock-up driver, so I am not sure what value there is in trying to
> balance devm_kzalloc() with corresponding devm_kfree()...
Example code should be as correct as possible.
^ permalink raw reply
* Re: [PATCH net] ipv6: make sure dev is not NULL before call ip6_frag_reasm
From: Eric Dumazet @ 2017-05-09 15:11 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev
In-Reply-To: <20170509134033.GB4649@leo.usersys.redhat.com>
On Tue, 2017-05-09 at 21:40 +0800, Hangbin Liu wrote:
>
> I saw we checked the dev in this function
>
> dev = skb->dev;
> if (dev) {
> fq->iif = dev->ifindex;
> skb->dev = NULL;
> }
>
> and upper caller ipv6_frag_rcv()
>
> fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
> skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
>
>
> Apologise that I did not do enough research to make sure whether skb->dev
> could be NULL or not. I will do the check recently and reply when got a
> confirmation.
If really having a NULL dev is possible, I would rather change things
this way, as your fix has side effects.
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index e1da5b888cc4901711d573075f8ae4eada7f086e..6c0a2b74ba705cbe13b4e7522d958a9c3d395c29 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -77,7 +77,7 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
static struct inet_frags ip6_frags;
static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
- struct net_device *dev);
+ struct net_device *dev, struct inet6_dev *idev);
/*
* callers should be careful not to use the hash value outside the ipfrag_lock
@@ -209,6 +209,7 @@ fq_find(struct net *net, __be32 id, const struct in6_addr *src,
static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
struct frag_hdr *fhdr, int nhoff)
{
+ struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
struct sk_buff *prev, *next;
struct net_device *dev;
int offset, end, fragsize;
@@ -223,8 +224,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
if ((unsigned int)end > IPV6_MAXPLEN) {
- __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
- IPSTATS_MIB_INHDRERRORS);
+ __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
((u8 *)&fhdr->frag_off -
skb_network_header(skb)));
@@ -258,8 +258,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
/* RFC2460 says always send parameter problem in
* this case. -DaveM
*/
- __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
- IPSTATS_MIB_INHDRERRORS);
+ __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
offsetof(struct ipv6hdr, payload_len));
return -1;
@@ -354,7 +353,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
unsigned long orefdst = skb->_skb_refdst;
skb->_skb_refdst = 0UL;
- res = ip6_frag_reasm(fq, prev, dev);
+ res = ip6_frag_reasm(fq, prev, dev, idev);
skb->_skb_refdst = orefdst;
return res;
}
@@ -365,8 +364,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
discard_fq:
inet_frag_kill(&fq->q, &ip6_frags);
err:
- __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
- IPSTATS_MIB_REASMFAILS);
+ __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASMFAILS);
kfree_skb(skb);
return -1;
}
@@ -381,7 +379,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
* the last and the first frames arrived and all the bits are here.
*/
static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
- struct net_device *dev)
+ struct net_device *dev, struct inet6_dev *idev)
{
struct net *net = container_of(fq->q.net, struct net, ipv6.frags);
struct sk_buff *fp, *head = fq->q.fragments;
@@ -505,9 +503,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
skb_postpush_rcsum(head, skb_network_header(head),
skb_network_header_len(head));
- rcu_read_lock();
- __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
- rcu_read_unlock();
+ __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASMOKS);
fq->q.fragments = NULL;
fq->q.fragments_tail = NULL;
return 1;
^ permalink raw reply related
* Re: DQL and TCQ_F_CAN_BYPASS destroy performance under virtualizaiton (Was: "Re: net_sched strange in 4.11")
From: Stefan Hajnoczi @ 2017-05-09 15:11 UTC (permalink / raw)
To: Anton Ivanov; +Cc: David S. Miller, netdev, Michael S. Tsirkin, jasowang
In-Reply-To: <27ae4e1c-7c6c-14c2-f3a4-9d0b1265d034@cambridgegreys.com>
[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]
On Tue, May 09, 2017 at 08:46:46AM +0100, Anton Ivanov wrote:
> I have figured it out. Two issues.
>
> 1) skb->xmit_more is hardly ever set under virtualization because the qdisc
> is usually bypassed because of TCQ_F_CAN_BYPASS. Once TCQ_F_CAN_BYPASS is
> set a virtual NIC driver is not likely see skb->xmit_more (this answers my
> "how does this work at all" question).
>
> 2) If that flag is turned off (I patched sched_generic to turn it off in
> pfifo_fast while testing), DQL keeps xmit_more from being set. If the driver
> is not DQL enabled xmit_more is never ever set. If the driver is DQL enabled
> the queue is adjusted to ensure xmit_more stops happening within 10-15 xmit
> cycles.
>
> That is plain *wrong* for virtual NICs - virtio, emulated NICs, etc. There,
> the BIG cost is telling the hypervisor that it needs to "kick" the packets.
> The cost of putting them into the vNIC buffers is negligible. You want
> xmit_more to happen - it makes between 50% and 300% (depending on vNIC
> design) difference. If there is no xmit_more the vNIC will immediately
> "kick" the hypervisor and try to signal that the packet needs to move
> straight away (as for example in virtio_net).
>
> In addition to that, the perceived line rate is proportional to this cost,
> so I am not sure that the current dql math holds. In fact, I think it does
> not - it is trying to adjust something which influences the perceived line
> rate.
>
> So - how do we turn BOTH bypass and DQL adjustment while under
> virtualization and set them to be "always qdisc" + "always xmit_more
> allowed"
>
> A.
>
> P.S. Cc-ing virtio maintainer
CCing Michael Tsirkin and Jason Wang, who are the core virtio and
virtio-net maintainers. (I maintain the vsock driver - it's unrelated
to this discussion.)
>
> A.
>
>
> On 08/05/17 08:15, Anton Ivanov wrote:
> > Hi all,
> >
> > I was revising some of my old work for UML to prepare it for submission
> > and I noticed that skb->xmit_more does not seem to be set any more.
> >
> > I traced the issue as far as net/sched/sched_generic.c
> >
> > try_bulk_dequeue_skb() is never invoked (the drivers I am working on are
> > dql enabled so that is not the problem).
> >
> > More interestingly, if I put a breakpoint and debug output into
> > dequeue_skb() around line 147 - right before the bulk: tag that skb
> > there is always NULL. ???
> >
> > Similarly, debug in pfifo_fast_dequeue shows only NULLs being dequeued.
> > Again - ???
> >
> > First and foremost, I apologize for the silly question, but how can this
> > work at all? I see the skbs showing up at the driver level, why are
> > NULLs being returned at qdisc dequeue and where do the skbs at the
> > driver level come from?
> >
> > Second, where should I look to fix it?
> >
> > A.
> >
>
>
> --
> Anton R. Ivanov
>
> Cambridge Greys Limited, England company No 10273661
> http://www.cambridgegreys.com/
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] hamradio: Fine-tuning for nine function implementations
From: David Miller @ 2017-05-09 14:43 UTC (permalink / raw)
To: elfring; +Cc: linux-hams, netdev, javier, jpr, linux-kernel, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
You can feel free to continue submitting these changes, even though
people have asked you to back off on this, and that there is little to
no value to this churn.
But I personally am not going to apply any of your changes...
Especially since you keep posting even though people are asking you to
not make these changes.
You can ignore feedback like that, and you are explicitly being
notified that as a result, we can feel free to ignore you _too_.
People who submit kernel changes in the way you do waste a lot of
people's valuable time which could be spent on fixing real bugs,
implementing new important features, adding new documentation to
improve the understanding of the kernel for everyone, etc.
But instead, that time is being invested to reviewing your extremely
low value patches, many of which are undesirable.
I will not stand for it as the networking maintainer and am going to
ignore everything you submit until your approach and attitude towards
kernel patch submission _fundamentally_ (not temporarily, or for one
specific set of patches) changes.
Thank you.
^ permalink raw reply
* RE: sky2: Use seq_putc() in sky2_debug_show()
From: David Laight @ 2017-05-09 14:26 UTC (permalink / raw)
To: 'Stephen Hemminger', SF Markus Elfring
Cc: Lino Sanfilippo, netdev@vger.kernel.org, Mirko Lindner, LKML,
kernel-janitors@vger.kernel.org
In-Reply-To: <20170508225012.508f24c0@xeon-e3>
From: Stephen Hemminger
> Sent: 09 May 2017 06:50
> On Mon, 8 May 2017 19:42:46 +0200
> SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>
> > > Which issue do you mean? I dont see any issue you fix here.
> >
> > Are the run time characteristics a bit nicer for the function seq_putc
> > in comparison to the function seq_puts for printing a single line break here?
> >
> > Regards,
> > Markus
>
> I would put this in why bother category. seq_puts is correct and this is only
> in diagnostic output useful to developer and disabled on most distro kernels
Sometimes consistency is best.
Output everything with seq_printf(), using a format "%s" if necessary.
The performance really doesn't matter here at all.
It is also (probably) possible to get gcc to do the conversions - as it does for printf().
(A right PITA for embedded systems where only printf() exists.)
David
^ permalink raw reply
* [PATCH 4/4] hamradio: Adjust four function calls together with a variable assignment
From: SF Markus Elfring @ 2017-05-09 14:25 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:57:17 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/bpqether.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index eaa0f2e8e561..5e234e0ca256 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -185,7 +185,8 @@ static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_ty
if (!net_eq(dev_net(dev), &init_net))
goto drop;
- if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!skb)
return NET_RX_DROP;
if (!pskb_may_pull(skb, sizeof(struct ethhdr)))
@@ -286,7 +287,8 @@ static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev)
bpq = netdev_priv(dev);
orig_dev = dev;
- if ((dev = bpq_get_ether_dev(dev)) == NULL) {
+ dev = bpq_get_ether_dev(dev);
+ if (!dev) {
orig_dev->stats.tx_dropped++;
kfree_skb(skb);
return NETDEV_TX_OK;
@@ -565,12 +567,14 @@ static int bpq_device_event(struct notifier_block *this,
break;
case NETDEV_DOWN: /* ethernet device closed -> close BPQ interface */
- if ((dev = bpq_get_ax25_dev(dev)) != NULL)
+ dev = bpq_get_ax25_dev(dev);
+ if (dev)
dev_close(dev);
break;
case NETDEV_UNREGISTER: /* ethernet device removed -> free BPQ interface */
- if ((dev = bpq_get_ax25_dev(dev)) != NULL)
+ dev = bpq_get_ax25_dev(dev);
+ if (dev)
bpq_free_device(dev);
break;
default:
--
2.12.2
^ permalink raw reply related
* Re: Marvell phy errata origins?
From: Andrew Lunn @ 2017-05-09 14:24 UTC (permalink / raw)
To: Daniel Walker
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
In-Reply-To: <ffd12ac5-7501-0310-182f-1735c2da8165@cisco.com>
> According to Marvell this was errata for 88M1101 , and should not be
> applied to any other phy .. So we should be removing these lines and
> make a special aneg for 88M1101 then restore everything that doesn't
> need this back to the generic aneg,
Hi Daniel
Thanks for finding this out. Can you role a patch?
Andrew
^ permalink raw reply
* [PATCH 3/4] hamradio: Use seq_puts() in bpq_seq_show()
From: SF Markus Elfring @ 2017-05-09 14:24 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:45:09 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/bpqether.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index f62e7f325cf9..eaa0f2e8e561 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -434,7 +434,7 @@ static int bpq_seq_show(struct seq_file *seq, void *v)
bpqdev->dest_addr);
if (is_multicast_ether_addr(bpqdev->acpt_addr))
- seq_printf(seq, "*\n");
+ seq_puts(seq, "*\n");
else
seq_printf(seq, "%pM\n", bpqdev->acpt_addr);
--
2.12.2
^ permalink raw reply related
* [PATCH 2/4] hamradio: Adjust four function calls together with a variable assignment
From: SF Markus Elfring @ 2017-05-09 14:23 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:15:16 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix affected source code places.
Improve a size determination.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/yam.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index 542f1e511df1..c792b0f116a5 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -401,7 +401,8 @@ static unsigned char *add_mcs(unsigned char *bits, int bitrate,
}
/* Allocate a new mcs */
- if ((p = kmalloc(sizeof(struct yam_mcs), GFP_KERNEL)) == NULL) {
+ p = kmalloc(sizeof(*p), GFP_KERNEL);
+ if (!p) {
release_firmware(fw);
return NULL;
}
@@ -549,7 +550,8 @@ static inline void yam_rx_flag(struct net_device *dev, struct yam_port *yp)
if ((yp->rx_crch & yp->rx_crcl) != 0xFF) {
/* Bad crc */
} else {
- if (!(skb = dev_alloc_skb(pkt_len))) {
+ skb = dev_alloc_skb(pkt_len);
+ if (!skb) {
printk(KERN_WARNING "%s: memory squeeze, dropping packet\n", dev->name);
++dev->stats.rx_dropped;
} else {
@@ -670,7 +672,8 @@ static void yam_tx_byte(struct net_device *dev, struct yam_port *yp)
break;
case TX_HEAD:
if (--yp->tx_count <= 0) {
- if (!(skb = skb_dequeue(&yp->send_queue))) {
+ skb = skb_dequeue(&yp->send_queue);
+ if (!skb) {
ptt_off(dev);
yp->tx_state = TX_OFF;
break;
@@ -879,7 +882,8 @@ static int yam_open(struct net_device *dev)
printk(KERN_ERR "%s: cannot 0x%lx busy\n", dev->name, dev->base_addr);
return -EACCES;
}
- if ((u = yam_check_uart(dev->base_addr)) == c_uart_unknown) {
+ u = yam_check_uart(dev->base_addr);
+ if (u == c_uart_unknown) {
printk(KERN_ERR "%s: cannot find uart type\n", dev->name);
ret = -EIO;
goto out_release_base;
--
2.12.2
^ permalink raw reply related
* [PATCH 1/4] hamradio: Combine two seq_printf() calls into one in yam_seq_show()
From: SF Markus Elfring @ 2017-05-09 14:22 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 14:16:45 +0200
A bit of data was put into a sequence by two separate function calls.
Print the same data by a single function call instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/yam.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index b6891ada1d7b..542f1e511df1 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -830,8 +830,7 @@ static int yam_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, " RxFrames %lu\n", dev->stats.rx_packets);
seq_printf(seq, " TxInt %u\n", yp->nb_mdint);
seq_printf(seq, " RxInt %u\n", yp->nb_rxint);
- seq_printf(seq, " RxOver %lu\n", dev->stats.rx_fifo_errors);
- seq_printf(seq, "\n");
+ seq_printf(seq, " RxOver %lu\n\n", dev->stats.rx_fifo_errors);
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH 0/4] hamradio: Fine-tuning for nine function implementations
From: SF Markus Elfring @ 2017-05-09 14:21 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 16:11:23 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Combine two seq_printf() calls into one in yam_seq_show()
Adjust four function calls together with a variable assignment
Use seq_puts() in bpq_seq_show()
Adjust four function calls together with a variable assignment
drivers/net/hamradio/bpqether.c | 14 +++++++++-----
drivers/net/hamradio/yam.c | 15 +++++++++------
2 files changed, 18 insertions(+), 11 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Rob Herring @ 2017-05-09 14:14 UTC (permalink / raw)
To: Baruch Siach
Cc: Adam Ford, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Johan Hedberg,
Gustavo Padovan, Marcel Holtmann, Sebastian Reichel, Wei Xu,
open list:BLUETOOTH DRIVERS, Eyal Reizer, netdev, Satish Patel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20170509044837.oje2tfodytyuuuur-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>
On Mon, May 8, 2017 at 11:48 PM, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> Hi Rob,
>
> On Mon, May 08, 2017 at 04:12:16PM -0500, Rob Herring wrote:
>> On Fri, May 5, 2017 at 9:51 AM, Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Sun, Apr 30, 2017 at 11:04 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >> On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
>> >>> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >>> > This series adds serdev support to the HCI LL protocol used on TI BT
>> >>> > modules and enables support on HiKey board with with the WL1835 module.
>> >>> > With this the custom TI UIM daemon and btattach are no longer needed.
>> >>>
>> >>> Without UIM daemon, what instruction do you use to load the BT firmware?
>> >>>
>> >>> I was thinking 'hciattach' but I was having trouble. I was hoping you
>> >>> might have some insight.
>> >>>
>> >>> hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
>> >>> returns a timeout.
>> >>>
>> >>> I modified my i.MX6 device tree per the binding documentation and
>> >>> setup the regulators and enable GPIO pins.
>> >>
>> >> If you configured everything correctly no userspace interaction is
>> >> required. The driver should request the firmware automatically once
>> >> you power up the bluetooth device.
>> >>
>> >> Apart from DT changes make sure, that the following options are
>> >> enabled and check dmesg for any hints.
>> >>
>> >> CONFIG_SERIAL_DEV_BUS
>> >> CONFIG_SERIAL_DEV_CTRL_TTYPORT
>> >> CONFIG_BT_HCIUART
>> >> CONFIG_BT_HCIUART_LL
>> >
>> > I have enabled those flags, and I have updated my device tree.
>> > I am testing this on an OMAP3630 (DM3730) board with a WL1283. I am
>> > getting a lot of timeout errors. I tested this against the original
>> > implemention I had in pdata-quirks.c using the ti-st driver, uim & and
>> > the btwilink driver.
>> >
>> > I pulled in some of the newer patches to enable the wl1283-st, but I
>> > am obviously missing something.
>> >
>> > I 58.717651] Bluetooth: hci0: Reading TI version information failed
>> > (-110)
>> > [ 58.724853] Bluetooth: hci0: download firmware failed, retrying...
>> > [ 60.957641] Bluetooth: hci0 command 0x1001 tx timeout
>> > [ 68.957641] Bluetooth: hci0: Reading TI version information failed
>> > (-110)
>> > [ 68.964843] Bluetooth: hci0: download firmware failed, retrying...
>> > [ 69.132171] Bluetooth: Unknown HCI packet type 06
>> > [ 69.138244] Bluetooth: Unknown HCI packet type 0c
>> > [ 69.143249] Bluetooth: Unknown HCI packet type 40
>> > [ 69.148498] Bluetooth: Unknown HCI packet type 20
>> > [ 69.153533] Bluetooth: Data length is too large
>> > [ 69.158569] Bluetooth: Unknown HCI packet type a0
>> > [ 69.163574] Bluetooth: Unknown HCI packet type 00
>> > [ 69.168731] Bluetooth: Unknown HCI packet type 00
>> > [ 69.173736] Bluetooth: Unknown HCI packet type 34
>> > [ 69.178924] Bluetooth: Unknown HCI packet type 91
>> > [ 71.197631] Bluetooth: hci0 command 0x1001 tx timeout
>> > [ 79.197662] Bluetooth: hci0: Reading TI version information failed (-110)
>>
>> There's a bug in serdev_device_write(), so if you have that function
>> you need either the fix I sent or the patch to make
>> serdev_device_writebuf atomic again. Both are on the linux-serial
>> list, but not in any tree yet.
>
> You refer to the patches below, right?
>
> [PATCH] tty: serdev: fix serdev_device_write return value,
> http://www.spinics.net/lists/linux-serial/msg26117.html
>
> [PATCH] serdev: Restore serdev_device_write_buf for atomic context,
> http://www.spinics.net/lists/linux-serial/msg26113.html
Yes, either one will fix the issue.
Rob
^ permalink raw reply
* Re: [PATCH v7 5/5] virtio_net: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 14:10 UTC (permalink / raw)
To: Netdev, LKML, David Miller, kernel-hardening
Cc: Jason A. Donenfeld, Michael S. Tsirkin, Jason Wang
In-Reply-To: <20170509135009.13751-6-Jason@zx2c4.com>
On Tue, May 9, 2017 at 3:50 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
(The next submission of this will take into account this + 1 here.
https://git.zx2c4.com/linux-dev/log/?h=jd/safe-skb-vec )
^ permalink raw reply
* Re: [PATCH v7 2/5] ipsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 14:09 UTC (permalink / raw)
To: Netdev, LKML, David Miller, kernel-hardening
Cc: Jason A. Donenfeld, Steffen Klassert, Herbert Xu
In-Reply-To: <20170509135009.13751-3-Jason@zx2c4.com>
(The next submission of this ipsec patch will have this rebased over
the latest upstream tree.
https://git.zx2c4.com/linux-dev/log/?h=jd/safe-skb-vec )
^ 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