* RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Hayes Wang @ 2016-11-25 6:31 UTC (permalink / raw)
To: Mark Lord, David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, nic_swsd,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <23e0c132-8844-0a34-3e0b-e412f76493ba-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
Mark Lord [mailto:mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org]
> Sent: Friday, November 25, 2016 12:44 AM
[...]
> The bad data in this case is ASCII:
>
> "SRC=m3400:/ TARGET=/m340"
>
> This data is what is seen in /run/mount/utab, a file that is read/written over NFS on
> each boot.
>
> "SRC=m3400:/ TARGET=/m3400 ROOT=/
> ATTRS=nolock,addr=192.168.8.1\n"
>
> But how does this ASCII data end up at offset zero of the rx buffer??
> Not possible -- this isn't even stale data, because only an rx_desc could
> be at that offset in that buffer.
>
> So even if this were a platform memory coherency issue, one should still
> never see ASCII data at the beginning of an rx buffer. The driver NEVER
> writes anything to the rx buffers. Only the USB hardware ever does.
>
> And only the r8152 dongle/driver exhibits this issue.
> Other USB dongles do not. They *might* still have such issues,
> but because they use software checksums, the bad packets are caught/rejected.
Do you test it by rebooting? Maybe you could try a patch
commit 93fe9b183840 ("r8152: reset the bmu"). However, it should
only occur for the first urb buffer after rx is reset. I don't
think you would reset the rx frequently, so the situation seems
to be different.
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: verify root pointer before dereferncing it
From: Cong Wang @ 2016-11-25 6:23 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David Miller, Jiri Pirko, Roi Dayan,
Linux Kernel Network Developers, Jiri Pirko, Or Gerlitz,
Cong Wang, John Fastabend, Alexei Starovoitov
In-Reply-To: <58378309.2090101@iogearbox.net>
On Thu, Nov 24, 2016 at 4:17 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
>
> I'm not sure if setting a dummy object for each affected classifier is
> making things better. Are you having this in mind as a target for -net?
>
> We do kfree_rcu() the head (tp->root) and likewise do we kfree_rcu() the
> tp immediately after the callback. The head object's lifetime for such
> classifiers is thus always bound to the tp lifetime. And besides that,
> nothing apart from get() checks whether it's actually really NULL (and
> that check in get() is odd anyway; some cls do it, some don't).
>
Excellent point.
I thought we should exclude any parallel readers when we call destroy(),
you are taking a different approach by observing we only have to exclude
readers when we really free them, this seems fine to me after a second
thought, because the RCU API should take care of races with readers so
as long as we free everything in RCU callback we are good. Hmm...
But I may miss something since I am not an RCU expert.
[...]
>
> (Btw, matchall is still broken besides this fix. mall_delete() sets the
> RCU_INIT_POINTER(head->filter, NULL), so that a mall_delete() plus
> mall_destroy() combo doesn't free head->filter twice, but doing that is
> racy with mall_classify() where head->filter is dereferenced unchecked.
> Requires additional fix.)
This seems due to matchall only has one filter per tp. But you don't need
to worry since readers never read a freed pointer, right?
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Stephen Rothwell @ 2016-11-25 6:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eli Cooper, netdev
In-Reply-To: <1480046044.8455.529.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Thu, 24 Nov 2016 19:54:04 -0800 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Could you now report :
>
> ethtool -k eth0
Features for eth0:
rx-checksumming: on
tx-checksumming: on
tx-checksum-ipv4: off [fixed]
tx-checksum-ip-generic: on
tx-checksum-ipv6: off [fixed]
tx-checksum-fcoe-crc: off [fixed]
tx-checksum-sctp: on
scatter-gather: on
tx-scatter-gather: on
tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: on
tx-tcp-segmentation: on
tx-tcp-ecn-segmentation: off [fixed]
tx-tcp-mangleid-segmentation: off
tx-tcp6-segmentation: on
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: on
tx-vlan-offload: on
ntuple-filters: off
receive-hashing: on
highdma: on [fixed]
rx-vlan-filter: on [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: off [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: on
tx-gre-csum-segmentation: on
tx-ipxip4-segmentation: on
tx-ipxip6-segmentation: on
tx-udp_tnl-segmentation: on
tx-udp_tnl-csum-segmentation: on
tx-gso-partial: on
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
l2-fwd-offload: off [fixed]
busy-poll: off [fixed]
hw-tc-offload: off [fixed]
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Hayes Wang @ 2016-11-25 6:11 UTC (permalink / raw)
To: Mark Lord, netdev@vger.kernel.org
Cc: nic_swsd, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <cdb64b21-a590-8c4f-50b1-757302d7b528@pobox.com>
Mark Lord [mailto:mlord@pobox.com]
> Sent: Thursday, November 24, 2016 11:25 PM
[...]
> x86 has near fully-coherent memory, so it is the "easy" platform
> to get things working on. But Linux supports a very diverse number
> of platforms, with varying degrees of cache/memory coherency,
> and it can be tricky for things to work correctly on all of them.
However, I have test iperf on raspberry pi v1 which you suggest
for more than one day. I still couldn't reproduce your issue.
> If you are testing with the driver as currently in 4.4.34,
> then you won't even notice when things are screwing up,
> because the driver just silently drops packets.
> Or it passes them on without noticing that they have bad data.
I only drop the packet silently when the rx descriptor outside
the urb buffer. Then, I check the rx descriptor before checking
the length of the packet.
> Here (attached) is the instrumented driver I am using here now.
> I suggest you use it or something similar when testing,
> and not the stock driver.
I would test it again with your driver.
[...]
> Also, unrelated, but inside r8152_submit_rx() there is this code:
>
> /* The rx would be stopped, so skip submitting */
> if (test_bit(RTL8152_UNPLUG, &tp->flags) ||
> !test_bit(WORK_ENABLE, &tp->flags)
> || !netif_carrier_ok(tp->netdev))
> return 0;
>
> If that "return 0" statement is ever executed, doesn't it result
> in the loss/leak of a buffer?
They would be found back by calling rtl_start_rx(), when the rx
is restarted.
Best Regards,
Hayes
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Eli Cooper @ 2016-11-25 6:05 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: netdev
In-Reply-To: <20161125134520.68ca0969@canb.auug.org.au>
Hi Stephen,
On 2016/11/25 10:45, Stephen Rothwell wrote:
> Hi Eli,
>
> On Fri, 25 Nov 2016 10:18:12 +0800 Eli Cooper <elicooper@gmx.com> wrote:
>> Sounds like TSO/GSO packets are not properly segmented and therefore
>> dropped.
>>
>> Could you first try turning off segmentation offloading for the tunnel
>> interface?
>> ethtool -K sit0 tso off gso off
> On Thu, 24 Nov 2016 18:30:14 -0800 Eric Dumazet <eric.dumazet@gmail.com>
>> You also could try to disable TSO and see if this makes a difference
>>
>> ethtool -K sixtofour0 tso off
> So turning off tso brings performance up to IPv4 levels ...
>
> Thanks for that, it solves my immediate problem.
I think this is similar to the bug I fixed in commit ae148b085876
("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()").
I can reproduce a similar problem by applying xfrm to sit traffic.
TSO/GSO packets are dropped when IPSec is enabled, and IPv6 throughput
drops to 10s of Kbps. I am not sure if this is the same issue you
experienced, but I wrote a patch that fixed at least the issue I had.
Could you test the patch I sent to the mailing list just now?
Thanks,
Eli
^ permalink raw reply
* [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: Eli Cooper @ 2016-11-25 5:50 UTC (permalink / raw)
To: netdev, David S . Miller, Stephen Rothwell
Similar to commit ae148b085876
("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()"),
sit tunnels also need to update skb->protocol; otherwise, TSO/GSO packets
might not be properly segmented, which causes the packets being dropped.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Tested-by: Eli Cooper <elicooper@gmx.com>
Cc: stable@vger.kernel.org
Signed-off-by: Eli Cooper <elicooper@gmx.com>
---
net/ipv6/sit.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index b1cdf80..a05dceb 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -972,6 +972,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
}
skb_set_inner_ipproto(skb, IPPROTO_IPV6);
+ skb->protocol = htons(ETH_P_IP);
iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, protocol, tos, ttl,
df, !net_eq(tunnel->net, dev_net(dev)));
--
2.10.2
^ permalink raw reply related
* Re: [scr265482] ip_tunnel.c
From: Liyang Yu (于立洋1) @ 2016-11-25 5:44 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Cong Wang, netdev@vger.kernel.org
>Bug is in the RFC for not describing how both ends would magically synchronize, with a sequence protocol which is unidirectional, with no ACK packets or whatever going back.
>With no description, it means that RFC author(s) never considered one side of the tunnel could die and restart.
>Contact the author(s) to get his thoughts maybe ?
I agree with you, but I don't know how to contacted the author(s). If any news about this, it will be grateful let me know.
>You will learn that not everything put in RFC or other piece of paper makes sense.
>And the implementation is exactly implementing the RFC bug, because, why not ?
>Now if you believe you can make this work, please send patches.
Now I needs to regain a sense of the RFC. Thank you.
I had thought about this issue, but GRE didn't encrypt the SEQ field, so it's difficult to let the other end to update the local SEQ, When the peer has restarted.
Because the attacker can hack the packet easily.
Maybe do not use the SEQ is a good choice.
In other words If the attacker reply the packets with sequence number bigger than the normal packets, can the tunnel work well? haw-haw, is this condition able to meet
a CVE.
On Thu, Nov 24, 2016 at 7:52 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
> I accept that the issue is not a CVE candidate. But it's a bug isn't
> it
Bug is in the RFC for not describing how both ends would magically synchronize, with a sequence protocol which is unidirectional, with no ACK packets or whatever going back.
With no description, it means that RFC author(s) never considered one side of the tunnel could die and restart.
Contact the author(s) to get his thoughts maybe ?
>
> Thank you for your suggestions, about the format of mail, and not next time.
>
> Everything has its meaning. If sequence number is a joke, why the guys put it into RFC , even implemented the feature.
You will learn that not everything put in RFC or other piece of paper makes sense.
And the implementation is exactly implementing the RFC bug, because, why not ?
Now if you believe you can make this work, please send patches.
>
> And if you means that SMP ( Symmetric Multi Processing )?
Yes.
If you enable sequences, GRE performance is abysmal on SMP, because linux uses an extra lock, instead of allowing multiple cpus using GRE tunnel at the same time, and GSO/TSO are disabled .
>
>
>
> On Thu, Nov 24, 2016 at 5:39 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>>
>>
>>
>>
>>
>>
>>
>> BTW:
>>
>> Which RFC suggests UINT_MAX as GRE sequence number? Can you show me?
>>
>>
>>
>>
>>
>>
>
>
> RFC 2890
>
> In any cases, this is absolutely not a security issue nor a CVE candidate.
> Please remove security@kernel.org from CC, no need to spam security guys, they have enough on their plate.
>
> Please send text messages, no HTML is allowed on netdev
>
> Nobody sane uses GRE sequence numbers, precisely because GRE has no documented way to synchronize the source and destination of the tunnel.
> Basically, if you use GRE sequence numbers, you must re-start other side of the tunnel if one side had to restart, or risk dropping up to 2^31 packets.
>
> Really, this is not something that can be solved by using 'a different initial sequence number'
>
> linux GRE sequence number support is a joke, it does not support SMP for a start.
>
>
>
>>
>>
>>
>> On Wed, Nov 23, 2016 at 11:45 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>>
>> > Yeah，I means that recreate the tunnel again, But I don’t think the
>>
>> > patch can fix the bug. It only can make the first packet received successed. And the follow packet will droped also.
>>
>> > In function __gre_xmit line 366
>>
>> > tunnel->o_seqno++;
>>
>> >
>>
>> > If you restart from UINT_MAX, the 'o_seqno' of second packet will return to 0 again.
>>
>>
>>
>> The first packet after restart: o_seqno == UINT_MAX, the other end:
>> i_seqno = 0 The second packet after restart: o_seqno == 0, the other
>> end: i_seqno = 1
>>
>>
>>
>> So traffic should be back to normal.
>>
>>
>>
>> UINT_MAX is also what RFC suggests.
^ permalink raw reply
* Re: [scr265482] ip_tunnel.c
From: Liyang Yu (于立洋1) @ 2016-11-25 5:43 UTC (permalink / raw)
To: Cong Wang; +Cc: Eric Dumazet, Linux Kernel Network Developers
Yeah you got it. Great!!!
On Thu, Nov 24, 2016 at 9:11 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>> Please test my patch since you can reproduce it.
>
> Thanks cong, but Eric had said: "Really, this is not something that can be solved by using 'a different initial sequence number'"
> So I don't think it's nessary to test you patch, anyway thank you very much.
That is correct, I missed the signed cast, it is similar to the jiffies test, for the overflow case, which means no matter which value we pick, we could drop packet as it is.
^ permalink raw reply
* Re: 答复: [scr265482] ip_tunnel.c
From: Cong Wang @ 2016-11-25 5:41 UTC (permalink / raw)
To: Liyang Yu (于立洋1)
Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <F41877A268BCDE49BAEE2286FA8C269F6FECFDBC@LETV-MBX-IDC09.letv.local>
On Thu, Nov 24, 2016 at 9:11 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>> Please test my patch since you can reproduce it.
>
> Thanks cong, but Eric had said: "Really, this is not something that can be solved by using 'a different initial sequence number'"
> So I don't think it's nessary to test you patch, anyway thank you very much.
That is correct, I missed the signed cast, it is similar to the jiffies test,
for the overflow case, which means no matter which value we pick,
we could drop packet as it is.
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: enable multiqueue by default
From: Jason Wang @ 2016-11-25 5:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Neil Horman, Jeremy Eder, Hannes Frederic Sowa, netdev,
linux-kernel, virtualization, Marko Myllynen, Maxime Coquelin
In-Reply-To: <20161125064201-mutt-send-email-mst@kernel.org>
On 2016年11月25日 12:43, Michael S. Tsirkin wrote:
> On Fri, Nov 25, 2016 at 12:37:26PM +0800, Jason Wang wrote:
>> >We use single queue even if multiqueue is enabled and let admin to
>> >enable it through ethtool later. This is used to avoid possible
>> >regression (small packet TCP stream transmission). But looks like an
>> >overkill since:
>> >
>> >- single queue user can disable multiqueue when launching qemu
>> >- brings extra troubles for the management since it needs extra admin
>> > tool in guest to enable multiqueue
>> >- multiqueue performs much better than single queue in most of the
>> > cases
>> >
>> >So this patch enables multiqueue by default: if #queues is less than or
>> >equal to #vcpu, enable as much as queue pairs; if #queues is greater
>> >than #vcpu, enable #vcpu queue pairs.
>> >
>> >Cc: Hannes Frederic Sowa<hannes@redhat.com>
>> >Cc: Michael S. Tsirkin<mst@redhat.com>
>> >Cc: Neil Horman<nhorman@redhat.com>
>> >Cc: Jeremy Eder<jeder@redhat.com>
>> >Cc: Marko Myllynen<myllynen@redhat.com>
>> >Cc: Maxime Coquelin<maxime.coquelin@redhat.com>
>> >Signed-off-by: Jason Wang<jasowang@redhat.com>
> OK at some level but all uses of num_online_cpus()
> like this are racy versus hotplug.
> I know we already have this bug but shouldn't we fix it
> before we add more?
Not sure I get the point, do you mean adding get/put_online_cpus()? But
is it a real bug? We don't do any cpu specific things so I believe it's
not necessary (unless we want to keep #queues == #vcpus magically but I
don't think so). Admin need to re-configure #queues after cpu hotplug if
they wish.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: enable multiqueue by default
From: Michael S. Tsirkin @ 2016-11-25 4:43 UTC (permalink / raw)
To: Jason Wang
Cc: Neil Horman, Jeremy Eder, Hannes Frederic Sowa, netdev,
linux-kernel, virtualization, Marko Myllynen, Maxime Coquelin
In-Reply-To: <1480048646-17536-1-git-send-email-jasowang@redhat.com>
On Fri, Nov 25, 2016 at 12:37:26PM +0800, Jason Wang wrote:
> We use single queue even if multiqueue is enabled and let admin to
> enable it through ethtool later. This is used to avoid possible
> regression (small packet TCP stream transmission). But looks like an
> overkill since:
>
> - single queue user can disable multiqueue when launching qemu
> - brings extra troubles for the management since it needs extra admin
> tool in guest to enable multiqueue
> - multiqueue performs much better than single queue in most of the
> cases
>
> So this patch enables multiqueue by default: if #queues is less than or
> equal to #vcpu, enable as much as queue pairs; if #queues is greater
> than #vcpu, enable #vcpu queue pairs.
>
> Cc: Hannes Frederic Sowa <hannes@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Neil Horman <nhorman@redhat.com>
> Cc: Jeremy Eder <jeder@redhat.com>
> Cc: Marko Myllynen <myllynen@redhat.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
OK at some level but all uses of num_online_cpus()
like this are racy versus hotplug.
I know we already have this bug but shouldn't we fix it
before we add more?
> ---
> drivers/net/virtio_net.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d4ac7a6..a21d93a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1886,8 +1886,11 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (vi->any_header_sg)
> dev->needed_headroom = vi->hdr_len;
>
> - /* Use single tx/rx queue pair as default */
> - vi->curr_queue_pairs = 1;
> + /* Enable multiqueue by default */
> + if (num_online_cpus() >= max_queue_pairs)
> + vi->curr_queue_pairs = max_queue_pairs;
> + else
> + vi->curr_queue_pairs = num_online_cpus();
> vi->max_queue_pairs = max_queue_pairs;
>
> /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
> @@ -1918,6 +1921,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> goto free_unregister_netdev;
> }
>
> + virtnet_set_affinity(vi);
> +
> /* Assume link up if device can't report link status,
> otherwise get link status from config. */
> if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
> --
> 2.7.4
^ permalink raw reply
* [PATCH net-next] virtio-net: enable multiqueue by default
From: Jason Wang @ 2016-11-25 4:37 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel
Cc: Neil Horman, Jeremy Eder, Michael S . Tsirkin,
Hannes Frederic Sowa, Marko Myllynen, Maxime Coquelin
We use single queue even if multiqueue is enabled and let admin to
enable it through ethtool later. This is used to avoid possible
regression (small packet TCP stream transmission). But looks like an
overkill since:
- single queue user can disable multiqueue when launching qemu
- brings extra troubles for the management since it needs extra admin
tool in guest to enable multiqueue
- multiqueue performs much better than single queue in most of the
cases
So this patch enables multiqueue by default: if #queues is less than or
equal to #vcpu, enable as much as queue pairs; if #queues is greater
than #vcpu, enable #vcpu queue pairs.
Cc: Hannes Frederic Sowa <hannes@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Neil Horman <nhorman@redhat.com>
Cc: Jeremy Eder <jeder@redhat.com>
Cc: Marko Myllynen <myllynen@redhat.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d4ac7a6..a21d93a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1886,8 +1886,11 @@ static int virtnet_probe(struct virtio_device *vdev)
if (vi->any_header_sg)
dev->needed_headroom = vi->hdr_len;
- /* Use single tx/rx queue pair as default */
- vi->curr_queue_pairs = 1;
+ /* Enable multiqueue by default */
+ if (num_online_cpus() >= max_queue_pairs)
+ vi->curr_queue_pairs = max_queue_pairs;
+ else
+ vi->curr_queue_pairs = num_online_cpus();
vi->max_queue_pairs = max_queue_pairs;
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
@@ -1918,6 +1921,8 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_unregister_netdev;
}
+ virtnet_set_affinity(vi);
+
/* Assume link up if device can't report link status,
otherwise get link status from config. */
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
--
2.7.4
^ permalink raw reply related
* Re: Large performance regression with 6in4 tunnel (sit)
From: Sven-Haegar Koch @ 2016-11-25 4:06 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Eli Cooper, netdev
In-Reply-To: <20161125134520.68ca0969@canb.auug.org.au>
On Fri, 25 Nov 2016, Stephen Rothwell wrote:
> On Fri, 25 Nov 2016 10:18:12 +0800 Eli Cooper <elicooper@gmx.com> wrote:
> >
> > Sounds like TSO/GSO packets are not properly segmented and therefore
> > dropped.
> >
> > Could you first try turning off segmentation offloading for the tunnel
> > interface?
> > ethtool -K sit0 tso off gso off
>
> On Thu, 24 Nov 2016 18:30:14 -0800 Eric Dumazet <eric.dumazet@gmail.com>
> >
> > You also could try to disable TSO and see if this makes a difference
> >
> > ethtool -K sixtofour0 tso off
>
> So turning off tso brings performance up to IPv4 levels ...
>
> Thanks for that, it solves my immediate problem.
Somehow this problem description really reminds me of a report on
netdev a bit ago, which the following patch fixed:
commit 9ee6c5dc816aa8256257f2cd4008a9291ec7e985
Author: Lance Richardson <lrichard@redhat.com>
Date: Wed Nov 2 16:36:17 2016 -0400
ipv4: allow local fragmentation in ip_finish_output_gso()
Some configurations (e.g. geneve interface with default
MTU of 1500 over an ethernet interface with 1500 MTU) result
in the transmission of packets that exceed the configured MTU.
While this should be considered to be a "bad" configuration,
it is still allowed and should not result in the sending
of packets that exceed the configured MTU.
Could this be related?
I suppose it would be difficult to test this patch on this machine?
c'ya
sven-haegar
--
Three may keep a secret, if two of them are dead.
- Ben F.
^ permalink raw reply
* Re: [scr265482] ip_tunnel.c
From: Eric Dumazet @ 2016-11-25 4:15 UTC (permalink / raw)
To: Liyang Yu (于立洋1); +Cc: Cong Wang, netdev@vger.kernel.org
In-Reply-To: <F41877A268BCDE49BAEE2286FA8C269F6FECFD83@LETV-MBX-IDC09.letv.local>
On Thu, Nov 24, 2016 at 7:52 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
> I accept that the issue is not a CVE candidate. But it's a bug isn't it
Bug is in the RFC for not describing how both ends would magically synchronize,
with a sequence protocol which is unidirectional, with no ACK packets
or whatever going back.
With no description, it means that RFC author(s) never considered one
side of the tunnel could die and restart.
Contact the author(s) to get his thoughts maybe ?
>
> Thank you for your suggestions, about the format of mail, and not next time.
>
> Everything has its meaning. If sequence number is a joke, why the guys put it into RFC , even implemented the feature.
You will learn that not everything put in RFC or other piece of paper
makes sense.
And the implementation is exactly implementing the RFC bug, because, why not ?
Now if you believe you can make this work, please send patches.
>
> And if you means that SMP ( Symmetric Multi Processing )?
Yes.
If you enable sequences, GRE performance is abysmal on SMP,
because linux uses an extra lock, instead of allowing multiple cpus
using GRE tunnel at the same time,
and GSO/TSO are disabled .
>
>
>
> On Thu, Nov 24, 2016 at 5:39 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>>
>>
>>
>>
>>
>>
>>
>> BTW:
>>
>> Which RFC suggests UINT_MAX as GRE sequence number? Can you show me?
>>
>>
>>
>>
>>
>>
>
>
> RFC 2890
>
> In any cases, this is absolutely not a security issue nor a CVE candidate.
> Please remove security@kernel.org from CC, no need to spam security guys, they have enough on their plate.
>
> Please send text messages, no HTML is allowed on netdev
>
> Nobody sane uses GRE sequence numbers, precisely because GRE has no documented way to synchronize the source and destination of the tunnel.
> Basically, if you use GRE sequence numbers, you must re-start other side of the tunnel if one side had to restart, or risk dropping up to 2^31 packets.
>
> Really, this is not something that can be solved by using 'a different initial sequence number'
>
> linux GRE sequence number support is a joke, it does not support SMP for a start.
>
>
>
>>
>>
>>
>> On Wed, Nov 23, 2016 at 11:45 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>>
>> > Yeah，I means that recreate the tunnel again, But I don’t think the
>>
>> > patch can fix the bug. It only can make the first packet received successed. And the follow packet will droped also.
>>
>> > In function __gre_xmit line 366
>>
>> > tunnel->o_seqno++;
>>
>> >
>>
>> > If you restart from UINT_MAX, the 'o_seqno' of second packet will return to 0 again.
>>
>>
>>
>> The first packet after restart: o_seqno == UINT_MAX, the other end:
>> i_seqno = 0 The second packet after restart: o_seqno == 0, the other
>> end: i_seqno = 1
>>
>>
>>
>> So traffic should be back to normal.
>>
>>
>>
>> UINT_MAX is also what RFC suggests.
^ permalink raw reply
* Re: [scr265482] ip_tunnel.c
From: Cong Wang @ 2016-11-25 4:12 UTC (permalink / raw)
To: Liyang Yu (于立洋1)
Cc: security@kernel.org, netdev@vger.kernel.org,
cve-request@mitre.org
In-Reply-To: <F41877A268BCDE49BAEE2286FA8C269F6FECFC5E@LETV-MBX-IDC09.letv.local>
On Thu, Nov 24, 2016 at 5:39 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
> BTW:
>
> Which RFC suggests UINT_MAX as GRE sequence number? Can you show me?
https://tools.ietf.org/html/rfc2890
"The receiver maintains the sequence number value of the last successfully
decapsulated packet. Upon establishment of the GRE tunnel, this value
should be set to (2**32)-1."
I agree with Eric here, this is a minor issue, not a serious bug at all.
Please test my patch since you can reproduce it.
BTW, please don't top-post here.
Thanks.
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Eric Dumazet @ 2016-11-25 3:54 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Eli Cooper, netdev
In-Reply-To: <20161125140938.24538f97@canb.auug.org.au>
On Fri, 2016-11-25 at 14:09 +1100, Stephen Rothwell wrote:
> Hi Eric,
>
> On Thu, 24 Nov 2016 19:01:28 -0800 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > Since I do not have this problem at all on my hosts, it could be a buggy
> > ethernet driver.
> >
> > Could you share what NIC card and driver you are using ?
>
>
> # uname -a
> Linux bilbo 4.7.0-1-amd64 #1 SMP Debian 4.7.8-1 (2016-10-19) x86_64 GNU/Linux
>
> # lspci | grep -i net
> 03:00.0 Ethernet controller: Intel Corporation I210 Gigabit Network Connection (rev 03)
> 04:00.0 Ethernet controller: Intel Corporation I210 Gigabit Network Connection (rev 03)
>
> from boot dmesg:
>
> [ 7.573725] igb: Intel(R) Gigabit Ethernet Network Driver - version 5.3.0-k
> [ 7.573726] igb: Copyright (c) 2007-2014 Intel Corporation.
> [ 7.752918] igb 0000:03:00.0: added PHC on eth0
> [ 7.752925] igb 0000:03:00.0: Intel(R) Gigabit Ethernet Network Connection
> [ 7.752927] igb 0000:03:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 00:1e:67:9f:d4:24
> [ 7.753460] igb 0000:03:00.0: eth0: PBA No: 102100-000
> [ 7.753460] igb 0000:03:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
> [ 7.902433] igb 0000:04:00.0: added PHC on eth1
> [ 7.902434] igb 0000:04:00.0: Intel(R) Gigabit Ethernet Network Connection
> [ 7.902435] igb 0000:04:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 00:1e:67:9f:d4:25
> [ 7.902484] igb 0000:04:00.0: eth1: PBA No: 102100-000
> [ 7.902485] igb 0000:04:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
> [ 19.753325] igb 0000:03:00.0 eth0: igb: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
>
Could you now report :
ethtool -k eth0
Thanks
^ permalink raw reply
* Re: [scr265482] ip_tunnel.c
From: Liyang Yu (于立洋1) @ 2016-11-25 3:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Cong Wang, netdev@vger.kernel.org
I accept that the issue is not a CVE candidate. But it's a bug isn't it
Thank you for your suggestions, about the format of mail, and not next time.
Everything has its meaning. If sequence number is a joke, why the guys put it into RFC , even implemented the feature.
And if you means that SMP ( Symmetric Multi Processing )?
On Thu, Nov 24, 2016 at 5:39 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>
>
>
>
>
>
>
> BTW:
>
> Which RFC suggests UINT_MAX as GRE sequence number? Can you show me?
>
>
>
>
>
>
RFC 2890
In any cases, this is absolutely not a security issue nor a CVE candidate.
Please remove security@kernel.org from CC, no need to spam security guys, they have enough on their plate.
Please send text messages, no HTML is allowed on netdev
Nobody sane uses GRE sequence numbers, precisely because GRE has no documented way to synchronize the source and destination of the tunnel.
Basically, if you use GRE sequence numbers, you must re-start other side of the tunnel if one side had to restart, or risk dropping up to 2^31 packets.
Really, this is not something that can be solved by using 'a different initial sequence number'
linux GRE sequence number support is a joke, it does not support SMP for a start.
>
>
>
> On Wed, Nov 23, 2016 at 11:45 PM, Liyang Yu (于立洋1) <yuliyang1@le.com> wrote:
>
> > Yeah，I means that recreate the tunnel again, But I don’t think the
>
> > patch can fix the bug. It only can make the first packet received successed. And the follow packet will droped also.
>
> > In function __gre_xmit line 366
>
> > tunnel->o_seqno++;
>
> >
>
> > If you restart from UINT_MAX, the 'o_seqno' of second packet will return to 0 again.
>
>
>
> The first packet after restart: o_seqno == UINT_MAX, the other end:
> i_seqno = 0 The second packet after restart: o_seqno == 0, the other
> end: i_seqno = 1
>
>
>
> So traffic should be back to normal.
>
>
>
> UINT_MAX is also what RFC suggests.
^ permalink raw reply
* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-25 3:49 UTC (permalink / raw)
To: Francois Romieu
Cc: David Miller, hayeswang, netdev, nic_swsd, linux-kernel,
linux-usb
In-Reply-To: <20161125002702.GA14085@electric-eye.fr.zoreil.com>
On 16-11-24 07:27 PM, Francois Romieu wrote:
>
> Through aliasing the URB was given a page that contains said (previously)
> received file. The ethernet chip/usb host does not write anything in it.
I don't see how that could be possible. Please elaborate.
The URB buffers are statically allocated by the driver at probe time,
ten of them in all, allocated with usb_alloc_coherent() in the copy of
the driver I am testing with.
There is no possibility for them to be used for anything other than
USB receive buffers, for this driver only. Nothing in the driver
or kernel ever writes to those buffers after initial allocation,
and only the driver and USB host controller ever have pointers to the buffers.
--
Mark Lord
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Stephen Rothwell @ 2016-11-25 3:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eli Cooper, netdev
In-Reply-To: <1480042888.8455.527.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Thu, 24 Nov 2016 19:01:28 -0800 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Since I do not have this problem at all on my hosts, it could be a buggy
> ethernet driver.
>
> Could you share what NIC card and driver you are using ?
# uname -a
Linux bilbo 4.7.0-1-amd64 #1 SMP Debian 4.7.8-1 (2016-10-19) x86_64 GNU/Linux
# lspci | grep -i net
03:00.0 Ethernet controller: Intel Corporation I210 Gigabit Network Connection (rev 03)
04:00.0 Ethernet controller: Intel Corporation I210 Gigabit Network Connection (rev 03)
from boot dmesg:
[ 7.573725] igb: Intel(R) Gigabit Ethernet Network Driver - version 5.3.0-k
[ 7.573726] igb: Copyright (c) 2007-2014 Intel Corporation.
[ 7.752918] igb 0000:03:00.0: added PHC on eth0
[ 7.752925] igb 0000:03:00.0: Intel(R) Gigabit Ethernet Network Connection
[ 7.752927] igb 0000:03:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 00:1e:67:9f:d4:24
[ 7.753460] igb 0000:03:00.0: eth0: PBA No: 102100-000
[ 7.753460] igb 0000:03:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
[ 7.902433] igb 0000:04:00.0: added PHC on eth1
[ 7.902434] igb 0000:04:00.0: Intel(R) Gigabit Ethernet Network Connection
[ 7.902435] igb 0000:04:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 00:1e:67:9f:d4:25
[ 7.902484] igb 0000:04:00.0: eth1: PBA No: 102100-000
[ 7.902485] igb 0000:04:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
[ 19.753325] igb 0000:03:00.0 eth0: igb: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Eric Dumazet @ 2016-11-25 3:01 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Eli Cooper, netdev
In-Reply-To: <20161125134520.68ca0969@canb.auug.org.au>
On Fri, 2016-11-25 at 13:45 +1100, Stephen Rothwell wrote:
> So turning off tso brings performance up to IPv4 levels ...
ok.
>
> Thanks for that, it solves my immediate problem.
Since I do not have this problem at all on my hosts, it could be a buggy
ethernet driver.
Could you share what NIC card and driver you are using ?
^ permalink raw reply
* Re: [PATCH net 1/1] driver: macvtap: Unregister netdev rx_handler if macvtap_newlink fails
From: Jason Wang @ 2016-11-25 2:48 UTC (permalink / raw)
To: fgao, davem, edumazet, netdev, gfree.wind
In-Reply-To: <1480039506-29740-1-git-send-email-fgao@ikuai8.com>
On 2016年11月25日 10:05, fgao@48lvckh6395k16k5.yundunddos.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
>
> The macvtap_newlink registers the netdev rx_handler firstly, but it
> does not unregister the handler if macvlan_common_newlink failed.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> drivers/net/macvtap.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 070e329..bceca28 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -491,7 +491,13 @@ static int macvtap_newlink(struct net *src_net,
> /* Don't put anything that may fail after macvlan_common_newlink
> * because we can't undo what it does.
> */
> - return macvlan_common_newlink(src_net, dev, tb, data);
> + err = macvlan_common_newlink(src_net, dev, tb, data);
> + if (err) {
> + netdev_rx_handler_unregister(dev);
> + return err;
> + }
> +
> + return 0;
> }
>
> static void macvtap_dellink(struct net_device *dev,
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Stephen Rothwell @ 2016-11-25 2:45 UTC (permalink / raw)
To: Eli Cooper; +Cc: netdev
In-Reply-To: <cf2fff0a-7d41-1416-0056-3718ea50a5bd@gmx.com>
Hi Eli,
On Fri, 25 Nov 2016 10:18:12 +0800 Eli Cooper <elicooper@gmx.com> wrote:
>
> Sounds like TSO/GSO packets are not properly segmented and therefore
> dropped.
>
> Could you first try turning off segmentation offloading for the tunnel
> interface?
> ethtool -K sit0 tso off gso off
On Thu, 24 Nov 2016 18:30:14 -0800 Eric Dumazet <eric.dumazet@gmail.com>
>
> You also could try to disable TSO and see if this makes a difference
>
> ethtool -K sixtofour0 tso off
So turning off tso brings performance up to IPv4 levels ...
Thanks for that, it solves my immediate problem.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: [PATCH 3/3] tools/virtio: use {READ,WRITE}_ONCE() in uaccess.h
From: Jason Wang @ 2016-11-25 2:40 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: dave, kvm, mst, netdev, dbueso, virtualization, paulmck, dvyukov
In-Reply-To: <1479983114-17190-4-git-send-email-mark.rutland@arm.com>
On 2016年11月24日 18:25, Mark Rutland wrote:
> As a step towards killing off ACCESS_ONCE, use {READ,WRITE}_ONCE() for the
> virtio tools uaccess primitives, pulling these in from <linux/compiler.h>.
>
> With this done, we can kill off the now-unused ACCESS_ONCE() definition.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> ---
> tools/virtio/linux/uaccess.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/virtio/linux/uaccess.h b/tools/virtio/linux/uaccess.h
> index 0a578fe..fa05d01 100644
> --- a/tools/virtio/linux/uaccess.h
> +++ b/tools/virtio/linux/uaccess.h
> @@ -1,8 +1,9 @@
> #ifndef UACCESS_H
> #define UACCESS_H
> -extern void *__user_addr_min, *__user_addr_max;
>
> -#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
> +#include <linux/compiler.h>
> +
> +extern void *__user_addr_min, *__user_addr_max;
>
> static inline void __chk_user_ptr(const volatile void *p, size_t size)
> {
> @@ -13,7 +14,7 @@ static inline void __chk_user_ptr(const volatile void *p, size_t size)
> ({ \
> typeof(ptr) __pu_ptr = (ptr); \
> __chk_user_ptr(__pu_ptr, sizeof(*__pu_ptr)); \
> - ACCESS_ONCE(*(__pu_ptr)) = x; \
> + WRITE_ONCE(*(__pu_ptr), x); \
> 0; \
> })
>
> @@ -21,7 +22,7 @@ static inline void __chk_user_ptr(const volatile void *p, size_t size)
> ({ \
> typeof(ptr) __pu_ptr = (ptr); \
> __chk_user_ptr(__pu_ptr, sizeof(*__pu_ptr)); \
> - x = ACCESS_ONCE(*(__pu_ptr)); \
> + x = READ_ONCE(*(__pu_ptr)); \
> 0; \
> })
>
Reviewed-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 2/3] vringh: kill off ACCESS_ONCE()
From: Jason Wang @ 2016-11-25 2:40 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: dave, kvm, mst, netdev, dbueso, virtualization, paulmck, dvyukov
In-Reply-To: <1479983114-17190-3-git-send-email-mark.rutland@arm.com>
On 2016年11月24日 18:25, Mark Rutland wrote:
> Despite living under drivers/ vringh.c is also used as part of the userspace
> virtio tools. Before we can kill off the ACCESS_ONCE()definition in the tools,
> we must convert vringh.c to use {READ,WRITE}_ONCE().
>
> This patch does so, along with the required include of <linux/compiler.h> for
> the relevant definitions. The userspace tools provide their own definitions in
> their own <linux/compiler.h>.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> ---
> drivers/vhost/vringh.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 3bb02c6..bb8971f 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -3,6 +3,7 @@
> *
> * Since these may be in userspace, we use (inline) accessors.
> */
> +#include <linux/compiler.h>
> #include <linux/module.h>
> #include <linux/vringh.h>
> #include <linux/virtio_ring.h>
> @@ -820,13 +821,13 @@ EXPORT_SYMBOL(vringh_need_notify_user);
> static inline int getu16_kern(const struct vringh *vrh,
> u16 *val, const __virtio16 *p)
> {
> - *val = vringh16_to_cpu(vrh, ACCESS_ONCE(*p));
> + *val = vringh16_to_cpu(vrh, READ_ONCE(*p));
> return 0;
> }
>
> static inline int putu16_kern(const struct vringh *vrh, __virtio16 *p, u16 val)
> {
> - ACCESS_ONCE(*p) = cpu_to_vringh16(vrh, val);
> + WRITE_ONCE(*p, cpu_to_vringh16(vrh, val));
> return 0;
> }
>
Reviewed-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] tools/virtio: fix READ_ONCE()
From: Jason Wang @ 2016-11-25 2:38 UTC (permalink / raw)
To: Mark Rutland, linux-kernel
Cc: dave, kvm, mst, netdev, dbueso, virtualization, paulmck, dvyukov
In-Reply-To: <1479983114-17190-2-git-send-email-mark.rutland@arm.com>
On 2016年11月24日 18:25, Mark Rutland wrote:
> The virtio tools implementation of READ_ONCE() has a single parameter called
> 'var', but erroneously refers to 'val' for its cast, and thus won't work unless
> there's a variable of the correct type that happens to be called 'var'.
>
> Fix this with s/var/val/, making READ_ONCE() work as expected regardless.
>
> Fixes: a7c490333df3cff5 ("tools/virtio: use virt_xxx barriers")
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> ---
> tools/virtio/linux/compiler.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h
> index 845960e..c9ccfd4 100644
> --- a/tools/virtio/linux/compiler.h
> +++ b/tools/virtio/linux/compiler.h
> @@ -4,6 +4,6 @@
> #define WRITE_ONCE(var, val) \
> (*((volatile typeof(val) *)(&(var))) = (val))
>
> -#define READ_ONCE(var) (*((volatile typeof(val) *)(&(var))))
> +#define READ_ONCE(var) (*((volatile typeof(var) *)(&(var))))
>
> #endif
Reviewed-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ 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