* Re: Handling an Extra Signal at PHY Reset
From: Thomas Petazzoni @ 2019-02-20 8:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: Paul Kocialkowski, Florian Fainelli, Heiner Kallweit, netdev,
Mylène Josserand
In-Reply-To: <20190219133629.GN14879@lunn.ch>
On Tue, 19 Feb 2019 14:36:29 +0100
Andrew Lunn <andrew@lunn.ch> wrote:
> This seems like an odd design. I've normally seen weak pull up/down
> resistors, not a switch, so i'm wondering why it is designed like
> this.
The key point here is that this "CONFIG" pin of the PHY is used during
reset to configure the PHY, but then once the reset sequence is
finished, this pin is used for PTP. From the datasheet, section 2.28.1
"PTP Control":
"""
To support the PTP Time Stamping function, the device has four pins
that are global to the entire PHY:
- PTP clock input pin (The CONFIG pin is used for this purpose.)
- PTP Event Request input pin (The LED[1] pin is used for this purpose)
- PTP Event Request input pin (The LED[1] pin is used for this purpose)
- Interrupt Pin (The LED[2] pin is used for this purpose)
"""
A bit further down in the datasheet:
"After configuration is completed and the external clock source is
enabled, the CONFIG pin is used as the external 125 Mhz reference clock
input"
So that's why our design as a switch: it allows the CONFIG pin to be
used for configuration during the reset sequence, and then as the pin
for the PTP clock input.
Does that clarify why the CONFIG pin is not simply connected to some
static pull-up/pull-down ?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net] mlxsw: __mlxsw_sp_port_headroom_set(): Fix a use of local variable
From: Ido Schimmel @ 2019-02-20 8:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Jiri Pirko, Petr Machata, mlxsw
In-Reply-To: <20190219.103338.1703676672870024906.davem@davemloft.net>
On Tue, Feb 19, 2019 at 10:33:38AM -0800, David Miller wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> Date: Tue, 19 Feb 2019 09:30:31 +0000
>
> > We have a series for net-next that adds support for Spectrum-2 shared
> > buffers and it depends on this patch. I was wondering if you could merge
> > net into net-next today or later this week, so that we could submit it.
> >
> > Normally I would just wait for the merge to happen, but it looks like
> > this is going to be the last week to submit changes and I prefer not to
> > miss the window while waiting for the inevitable merge.
> >
> > Thanks and sorry about the noise.
>
> Ido, I will send Linus a pull request today and he should pull it in and
> I should therefore be able to get net-next sync'd with it by the end of
> tomorrow.
Great, thanks!
^ permalink raw reply
* Re: [RFC PATCH net-next v3 05/21] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-02-20 8:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David Miller, Andrew Lunn, Jiri Pirko, linux-kernel
In-Reply-To: <20190219182759.0abf08ae@cakuba.netronome.com>
On Tue, Feb 19, 2019 at 06:27:59PM -0800, Jakub Kicinski wrote:
> On Mon, 18 Feb 2019 19:21:49 +0100 (CET), Michal Kubecek wrote:
> > + else if (is_u32)
> > + bitmap_from_arr32(val, bitmap, nbits);
> > + else
> > + bitmap_copy(val, bitmap, nbits);
> > + nla_for_each_nested(bit_attr, tb[ETHA_BITSET_BITS], rem) {
> > + *err = ethnl_update_bit(val, mask, nbits, bit_attr,
> > + is_list, names, legacy, info);
> > + if (*err < 0)
> > + goto out_free;
> > + }
> > + if (bitmask)
> > + __bitmap_to_any(bitmask, mask, nbits, is_u32);
> > + } else {
> > + unsigned int change_words = DIV_ROUND_UP(change_bits, 32);
> > +
> > + *err = 0;
> > + if (change_bits == 0 && tb[ETHA_BITSET_MASK])
> > + goto out_free;
> > + *err = -EINVAL;
> > + if (!tb[ETHA_BITSET_VALUE])
> > + goto out_free;
>
> !tb[ETHA_BITSET_BITS] && !tb[ETHA_BITSET_VALUE] is already rejected
> above.
Thank you. I'll drop this test.
Michal
^ permalink raw reply
* [PATCH net v2] team: use operstate consistently for linkup
From: George Wilkie @ 2019-02-20 8:19 UTC (permalink / raw)
To: Jiri Pirko, David S. Miller; +Cc: netdev
When a port is added to a team, its initial state is derived
from netif_carrier_ok rather than netif_oper_up.
If it is carrier up but operationally down at the time of being
added, the port state.linkup will be set prematurely.
port state.linkup should be set consistently using
netif_oper_up rather than netif_carrier_ok.
Fixes: f1d22a1e0595 ("team: account for oper state")
Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
---
drivers/net/team/team.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 958f1cf67282..6ce3f666d142 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1256,7 +1256,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
list_add_tail_rcu(&port->list, &team->port_list);
team_port_enable(team, port);
__team_compute_features(team);
- __team_port_change_port_added(port, !!netif_carrier_ok(port_dev));
+ __team_port_change_port_added(port, !!netif_oper_up(port_dev));
__team_options_change_check(team);
netdev_info(dev, "Port device %s added\n", portname);
@@ -2915,7 +2915,7 @@ static int team_device_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (netif_carrier_ok(dev))
+ if (netif_oper_up(dev))
team_port_change_check(port, true);
break;
case NETDEV_DOWN:
--
2.11.0
^ permalink raw reply related
* [PATCH net] net: ip6_gre: fix possible NULL pointer dereference in ip6erspan_set_version
From: Lorenzo Bianconi @ 2019-02-20 8:23 UTC (permalink / raw)
To: davem; +Cc: netdev, syzbot+30191cf1057abd3064af
In-Reply-To: <cover.1550620650.git.lorenzo.bianconi@redhat.com>
Fix a possible NULL pointer dereference in ip6erspan_set_version checking
nlattr data pointer
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 7549 Comm: syz-executor432 Not tainted 5.0.0-rc6-next-20190218
#37
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:ip6erspan_set_version+0x5c/0x350 net/ipv6/ip6_gre.c:1726
Code: 07 38 d0 7f 08 84 c0 0f 85 9f 02 00 00 49 8d bc 24 b0 00 00 00 c6 43
54 01 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 9a 02 00 00 4d 8b ac 24 b0 00 00 00 4d 85 ed 0f
RSP: 0018:ffff888089ed7168 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff8880869d6e58 RCX: 0000000000000000
RDX: 0000000000000016 RSI: ffffffff862736b4 RDI: 00000000000000b0
RBP: ffff888089ed7180 R08: 1ffff11010d3adcb R09: ffff8880869d6e58
R10: ffffed1010d3add5 R11: ffff8880869d6eaf R12: 0000000000000000
R13: ffffffff8931f8c0 R14: ffffffff862825d0 R15: ffff8880869d6e58
FS: 0000000000b3d880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000184 CR3: 0000000092cc5000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
ip6erspan_newlink+0x66/0x7b0 net/ipv6/ip6_gre.c:2210
__rtnl_newlink+0x107b/0x16c0 net/core/rtnetlink.c:3176
rtnl_newlink+0x69/0xa0 net/core/rtnetlink.c:3234
rtnetlink_rcv_msg+0x465/0xb00 net/core/rtnetlink.c:5192
netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2485
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5210
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1925
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xdd/0x130 net/socket.c:631
___sys_sendmsg+0x806/0x930 net/socket.c:2136
__sys_sendmsg+0x105/0x1d0 net/socket.c:2174
__do_sys_sendmsg net/socket.c:2183 [inline]
__se_sys_sendmsg net/socket.c:2181 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440159
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fffa69156e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440159
RDX: 0000000000000000 RSI: 0000000020001340 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 0000000000000001 R09: 00000000004002c8
R10: 0000000000000011 R11: 0000000000000246 R12: 00000000004019e0
R13: 0000000000401a70 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
---[ end trace 09f8a7d13b4faaa1 ]---
RIP: 0010:ip6erspan_set_version+0x5c/0x350 net/ipv6/ip6_gre.c:1726
Code: 07 38 d0 7f 08 84 c0 0f 85 9f 02 00 00 49 8d bc 24 b0 00 00 00 c6 43
54 01 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f
85 9a 02 00 00 4d 8b ac 24 b0 00 00 00 4d 85 ed 0f
RSP: 0018:ffff888089ed7168 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff8880869d6e58 RCX: 0000000000000000
RDX: 0000000000000016 RSI: ffffffff862736b4 RDI: 00000000000000b0
RBP: ffff888089ed7180 R08: 1ffff11010d3adcb R09: ffff8880869d6e58
R10: ffffed1010d3add5 R11: ffff8880869d6eaf R12: 0000000000000000
R13: ffffffff8931f8c0 R14: ffffffff862825d0 R15: ffff8880869d6e58
FS: 0000000000b3d880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000184 CR3: 0000000092cc5000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Fixes: 4974d5f678ab ("net: ip6_gre: initialize erspan_ver just for erspan tunnels")
Reported-and-tested-by: syzbot+30191cf1057abd3064af@syzkaller.appspotmail.com
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
net/ipv6/ip6_gre.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 43890898b0b5..4b9dd5e88329 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1722,6 +1722,9 @@ static int ip6erspan_tap_validate(struct nlattr *tb[], struct nlattr *data[],
static void ip6erspan_set_version(struct nlattr *data[],
struct __ip6_tnl_parm *parms)
{
+ if (!data)
+ return;
+
parms->erspan_ver = 1;
if (data[IFLA_GRE_ERSPAN_VER])
parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 0/2] ARM: dts: am335x-evm/evmsk: Fix PHY mode for ethernet
From: Peter Ujfalusi @ 2019-02-20 8:30 UTC (permalink / raw)
To: Tony Lindgren
Cc: bcousson, linux-omap, devicetree, linux-arm-kernel, nsekhar,
grygorii.strashko, vkoul, netdev, f.fainelli, marc.w.gonzalez,
niklas.cassel
In-Reply-To: <20190219165231.GK15711@atomide.com>
Hi Tony,
On 19/02/2019 18.52, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [190219 08:02]:
>> On 18/02/2019 18.26, Tony Lindgren wrote:
>>> OK so sounds like these are OK to wait for v5.1 merge window
>>> then as the dts changes alone won't fix anything?
>>
>> I think it would be better to send these to 5.0 to avoid the regression
>> siting in linux-next.
>>
>> The patches are actually doing to the correct thing (we need rgmii-id,
>> not rmgii-txid for these boards).
>> It was just a matter of luck that it worked with rgmii-txid as the
>> driver was broken and did not disabled the rxid when it should have.
>
> OK applying into omap-for-v5.0/fixes-v2. I'll send a pull
> request on Thursday most likely then, let's see if it makes
> it for v5.0.
Thank you!
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [PATCH net-next] net: udp Allow CHECKSUM_UNNECESSARY packets to do GRO.
From: maowenan @ 2019-02-20 8:32 UTC (permalink / raw)
To: Tom Herbert
Cc: Linux Kernel Network Developers, David S. Miller, Eric Dumazet
In-Reply-To: <CALx6S37fpFimxrQ1hfvhDs7ukOvNKmm1jHYmx00APYfif3-OMQ@mail.gmail.com>
On 2019/1/31 12:33, Tom Herbert wrote:
> On Wed, Jan 30, 2019 at 6:59 PM maowenan <maowenan@huawei.com> wrote:
>>
>>
>>
>> On 2019/1/31 10:43, Tom Herbert wrote:
>>> On Wed, Jan 30, 2019 at 5:58 PM maowenan <maowenan@huawei.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2019/1/30 4:24, Tom Herbert wrote:
>>>>> On Tue, Jan 29, 2019 at 12:08 AM maowenan <maowenan@huawei.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2019/1/29 14:24, Tom Herbert wrote:
>>>>>>> On Mon, Jan 28, 2019 at 10:04 PM maowenan <maowenan@huawei.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2019/1/29 12:01, Tom Herbert wrote:
>>>>>>>>> On Mon, Jan 28, 2019 at 7:00 PM maowenan <maowenan@huawei.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>> Do you have any comments about this change?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2019/1/23 11:33, Mao Wenan wrote:
>>>>>>>>>>> When udp4_gro_receive() get one packet that uh->check=0,
>>>>>>>>>>> skb_gro_checksum_validate_zero_check() will set the
>>>>>>>>>>> skb->ip_summed = CHECKSUM_UNNECESSARY;
>>>>>>>>>>> skb->csum_level = 0;
>>>>>>>>>>> Then udp_gro_receive() will flush the packet which is not CHECKSUM_PARTIAL,
>>>>>>>>>>> It is not our expect, because check=0 in udp header indicates this
>>>>>>>>>>> packet is no need to caculate checksum, we should go further to do GRO.
>>>>>>>>>>>
>>>>>>>>>>> This patch changes the value of csum_cnt according to skb->csum_level.
>>>>>>>>>>> ---
>>>>>>>>>>> include/linux/netdevice.h | 1 +
>>>>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>>>>>>>>> index 1377d08..9c819f1 100644
>>>>>>>>>>> --- a/include/linux/netdevice.h
>>>>>>>>>>> +++ b/include/linux/netdevice.h
>>>>>>>>>>> @@ -2764,6 +2764,7 @@ static inline void skb_gro_incr_csum_unnecessary(struct sk_buff *skb)
>>>>>>>>>>> * during GRO. This saves work if we fallback to normal path.
>>>>>>>>>>> */
>>>>>>>>>>> __skb_incr_checksum_unnecessary(skb);
>>>>>>>>>>> + NAPI_GRO_CB(skb)->csum_cnt = skb->csum_level + 1;
>>>>>>>>>
>>>>>>>>> That doesn't look right. This would be reinitializing the GRO
>>>>>>>>> checksums from the beginning.
>>>>>>>>>
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> I assume the code is bailing on this conditional:
>>>>>>>>>
>>>>>>>>> if (NAPI_GRO_CB(skb)->encap_mark ||
>>>>>>>>> (skb->ip_summed != CHECKSUM_PARTIAL &&
>>>>>>>>> NAPI_GRO_CB(skb)->csum_cnt == 0 &&
>>>>>>>>> !NAPI_GRO_CB(skb)->csum_valid) ||
>>>>>>>>> !udp_sk(sk)->gro_receive)
>>>>>>>>> goto out_unlock;
>>>>>>>>>
>>>>>>>>> I am trying to remember why this needs to check csum_cnt. If there was
>>>>>>>>> a csum_cnt for the UDP csum being zero from checksum-unnecessary, it
>>>>>>>>> was consumed by skb_gro_checksum_validate_zero_check in UDP4 GRO
>>>>>>>>> received.
>>>>>>>>
>>>>>>>> We have met the scene about two VMs in different host with vxlan packets, when udp4_gro_receive receives
>>>>>>>> one packet with ip_summed=CHECKSUM_NONE,csum_cnt=0,csum_valid=0,and udp->check=0, then skb_gro_checksum_validate_zero_check()->
>>>>>>>> skb_gro_incr_csum_unnecessary() validate it and set ip_summed=CHECKSUM_UNNECESSARY,csum_level=0, but csum_cnt and csum_valid
>>>>>>>> keep zero value. Then it will be flushed in udp_gro_receive(), the codes as you have showed.
>>>>>>>>
>>>>>>>> so I think it forgets to modify csum_cnt since csum_level is changed in skb_gro_incr_csum_unnecessary()->__skb_incr_checksum_unnecessary().
>>>>>>>>
>>>>>>> Yes, but the csum_level is changing since we've gone beyond the
>>>>>>> checksums initially reported inc checksum-unnecessary. GRO csum_cnt is
>>>>>>> initialized to skb->csum_level + 1 at the start of GRO processing.
>>>>>>>
>>>>>>> If I recall, the rule is that UDP GRO requires at least one non-zero
>>>>>>> checksum to be verified. The idea is that if we end up computing
>>>>>>> packet checksums on the host for inner checksums like TCP during GRO,
>>>>>>> then that's negating the performance benefits of GRO. Had UDP check
>>>>>>> not been zero then we would do checksum unnecessary conversion and so
>>>>>>> csum_valid would be set for the remainded of GRO processing. The
>>>>>>> existing code is following the rule I believe, so this may be working
>>>>>>> as intended.
>>>>>>
>>>>>> Do you have any suggestion if I need do GRO as udp->check is zero?
>>>>>> My previous modification which works fine as below:
>>>>>> if (NAPI_GRO_CB(skb)->encap_mark ||
>>>>>> (skb->ip_summed != CHECKSUM_PARTIAL &&
>>>>>> + skb->ip_summed != CHECKSUM_UNNECESSARY &&
>>>>>
>>>>> That's effectively disabling the rule that we need a real checksum
>>>>> calculation to proceed with GRO. Besides that, the device returning
>>>>> one checksum-unnecessary level because UDP csum is zero is pretty
>>>>> pointelss; we can just as easily deduce get to same state just by
>>>>> looking at the field with CHECKSUM_NONE. What we really want to see
>>>>> for GRO is a real checksum computation being done on the packet.
>>>>>
>>>>> A few questions:
>>>>>
>>>>> What type of packets are being GROed? Are these TCP? What performance
>>>>> difference do you see with our patch? Can you try enabling UDP
>>>>> checksums, and even RCO with VXLAN? With UDP encapsulation we
>>>>> generally see better performance with checksum enabled since UDP
>>>>> checksum offload is ubiquitous and we can easily convert
>>>>> checksum-unnecessary (with non-zero csum) to checksum-complete.
>>>>
>>>> We use the physical network card calculate the checksum of the inner packet with checksum offload.
>>>> Set the udp checksum of the vxlan header is 0.
>>>>
>>> I see. It sounds like the device is really verifying two checksums in
>>> the packet, the outer UDP checksum (which is zero for UDP) and an
>>> inner checksum, but only reporting one checksum was verified. The
>>> driver needs to set csum_level to 1 in this case (meaning two
>>> checksums have been verified for checksum-unnecessary). What NIC are
>>> you using?
>>
>> Currently it is 82599, whose driver can't recognize the vxlan packet.
>> I guess so many NICs can't do this checking in it's driver, so I think this is a
>> common case, will we fix it in stack?
>>
> Mao,
>
> The problem isn't in the stack, it seems to be in the driver. If the
> device reports a verified checksum for an encpasulated packet then the
> driver needs to set csum_level to 1. Otherwise, the stack can't just
> assume that the inner checksum was verified. *How* a driver deduces
> that the device is reporting about an encapsulated checksum is
> specific to the device and its driver. I'm not sure which driver your
> running, but if you search the code there should be something like
> "skb->csum_level =1" that would be a clue about support. A good
> example is ixgbe, if the device reports checksum verified and that
> packet was VXLAN, it deduces that the inner checksum was verified and
> so the driver sets CHECKSUM_UNNECESSARY and skb->csum_level. Of course
> all this complexity goes away when devices just provide
> checksum-complete.
>
> Tom
Hi Tom,
I have checked code in ixgbe_main.c ixgbe_rx_checksum(). UDP frames with a 0 checksum can be marked as
checksum errors. If it returns here, skb->ip_summed will be set to CHECKSUM_NONE.
Then the packet will be flush to stack in udp_gro_receive().
Do you think whether it is the fault of driver or not?
ixgbe_rx_checksum():
if (ixgbe_test_staterr(rx_desc, IXGBE_RXDADV_ERR_TCPE)) {
/*
* 82599 errata, UDP frames with a 0 checksum can be marked as
* checksum errors.
*/
if ((pkt_info & cpu_to_le16(IXGBE_RXDADV_PKTTYPE_UDP)) &&
test_bit(__IXGBE_RX_CSUM_UDP_ZERO_ERR, &ring->state))
return;
ring->rx_stats.csum_err++;
return;
}
udp_gro_receive():
if (NAPI_GRO_CB(skb)->encap_mark ||
(skb->ip_summed != CHECKSUM_PARTIAL &&
NAPI_GRO_CB(skb)->csum_cnt == 0 &&
!NAPI_GRO_CB(skb)->csum_valid) ||
!udp_sk(sk)->gro_receive)
goto out_unlock;
>
>>>
>>> Tom
>>>
>>>
>>>> With this patch, the bandwidth of TCP between two VMs increase from 2Gbit/s to 6Gbit/s.
>>>>
>>>>>
>>>>> Tom
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> NAPI_GRO_CB(skb)->csum_cnt == 0 &&
>>>>>> !NAPI_GRO_CB(skb)->csum_valid) ||
>>>>>> !udp_sk(sk)->gro_receive)
>>>>>> goto out_unlock;
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Tom
>>>>>>>
>>>>>>>>>
>>>>>>>>> .
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>> .
>>>
>>
>
> .
>
^ permalink raw reply
* [PATCH net-next] selftest/tls: Add test to verify received 'type' of non-data record
From: Vakul Garg @ 2019-02-20 8:43 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: borisp@mellanox.com, aviadye@mellanox.com, davejwatson@fb.com,
davem@davemloft.net, doronrk@fb.com, Vakul Garg
Test case 'control_msg' has been updated to peek non-data record and
then verify the type of record received. Subsequently, the same record
is retrieved without MSG_PEEK flag in recvmsg().
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
tools/testing/selftests/net/tls.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 4ac50ccb3272..47ddfc154036 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -753,6 +753,20 @@ TEST_F(tls, control_msg)
EXPECT_EQ(recv(self->cfd, buf, send_len, 0), -1);
vec.iov_base = buf;
+ EXPECT_EQ(recvmsg(self->cfd, &msg, MSG_WAITALL | MSG_PEEK), send_len);
+
+ cmsg = CMSG_FIRSTHDR(&msg);
+ EXPECT_NE(cmsg, NULL);
+ EXPECT_EQ(cmsg->cmsg_level, SOL_TLS);
+ EXPECT_EQ(cmsg->cmsg_type, TLS_GET_RECORD_TYPE);
+ record_type = *((unsigned char *)CMSG_DATA(cmsg));
+ EXPECT_EQ(record_type, 100);
+ EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+
+ /* Recv the message again without MSG_PEEK */
+ record_type = 0;
+ memset(buf, 0, sizeof(buf));
+
EXPECT_EQ(recvmsg(self->cfd, &msg, MSG_WAITALL), send_len);
cmsg = CMSG_FIRSTHDR(&msg);
EXPECT_NE(cmsg, NULL);
--
2.13.6
^ permalink raw reply related
* Re: linux-next: Fixes tag needs some work in the net-next tree
From: Jiri Pirko @ 2019-02-20 8:36 UTC (permalink / raw)
To: David Miller; +Cc: vkoul, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190219.224813.723564128666927693.davem@davemloft.net>
Wed, Feb 20, 2019 at 07:48:13AM CET, davem@davemloft.net wrote:
>From: Vinod Koul <vkoul@kernel.org>
>Date: Wed, 20 Feb 2019 10:10:55 +0530
>
>> On 20-02-19, 09:31, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> In commit
>>>
>>> a968b5e9d587 ("net: dsa: qca8k: Enable delay for RGMII_ID mode")
>>>
>>> Fixes tag
>>>
>>> Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")
>>>
>>> has these problem(s):
>>>
>>> - Target SHA1 does not exist
>>>
>>> Did you mean:
>>>
>>> Fixes: 5ecdd77c61c8 ("net: dsa: qca8k: disable delay for RGMII mode")
>>
>> Yes looks like I messed up the commit id.. Not sure why :(
>>
>> Dave would you like to drop this and me sending updated patch or
>> something else..
>>
>> Sorry about the miss
>
>Doesn't work that way, something in my tree is there forever.
Would be good to have some robot checking "Fixes" sanity...
^ permalink raw reply
* Re: [PATCH net v2] team: use operstate consistently for linkup
From: Jiri Pirko @ 2019-02-20 8:36 UTC (permalink / raw)
To: George Wilkie; +Cc: David S. Miller, netdev
In-Reply-To: <20190220081911.803-1-gwilkie@vyatta.att-mail.com>
Wed, Feb 20, 2019 at 09:19:11AM CET, gwilkie@vyatta.att-mail.com wrote:
>When a port is added to a team, its initial state is derived
>from netif_carrier_ok rather than netif_oper_up.
>If it is carrier up but operationally down at the time of being
>added, the port state.linkup will be set prematurely.
>port state.linkup should be set consistently using
>netif_oper_up rather than netif_carrier_ok.
>
>Fixes: f1d22a1e0595 ("team: account for oper state")
>Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink
From: Jiri Pirko @ 2019-02-20 8:37 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, oss-drivers, mkubecek, andrew
In-Reply-To: <20190219164926.23981359@cakuba.netronome.com>
Wed, Feb 20, 2019 at 01:49:26AM CET, jakub.kicinski@netronome.com wrote:
>On Tue, 19 Feb 2019 10:19:42 +0100, Jiri Pirko wrote:
>> Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@netronome.com wrote:
>> >On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote:
>> >> > static const struct ethtool_ops nfp_net_ethtool_ops = {
>> >>
>> >> Why don't you use the compat fallback? I think you should.
>> >
>> >You and Michal both asked the same so let me answer the first to ask :)
>> >- if devlink is built as a module the fallback is not reachable.
>>
>> So the fallback is not really good as you can't use it for real drivers
>> anyway. Odd. Maybe we should compile devlink in without possibility to
>> have it as module.
>
>Ack, I'll make devlink a bool.
Thanks!
>
>I need a little extra time, I forgot that nfp's flower offload still
>doesn't register all ports (using your port flavour infrastructure).
Finally :)
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH] net: dsa: add missing of_node_put
From: Himadri Pandya @ 2019-02-20 9:10 UTC (permalink / raw)
To: Vaishali Thakkar
Cc: outreachy-kernel, Julia Lawall, andrew, Florian Fainelli,
David S. Miller, netdev, Linux Kernel Mailing List
In-Reply-To: <CAK-LDb+pTxoeuVYb+V6N6everYRtOvJwo26O-=LVVEbsXqBOFA@mail.gmail.com>
On 20/02/19 9:23 AM, Vaishali Thakkar wrote:
> On Wed, Feb 20, 2019 at 8:54 AM Himadri Pandya <himadri18.07@gmail.com> wrote:
> Hi Himadri,
>
> Thanks for the patch!
>
> For the scope of Outreachy, we prefer that you send patches in staging
> directory as Greg makes sure to pick them during the application
> period. Of course, you're very much encouraged to contribute to other
> subsystems as well but there patches are mainly picked up based on
> maintainer's cycle which may or may not be picked up for linux-next when
> selection of interns happens.
>
> I hope that makes sense.
Understood. Thank you for letting me know that.
>> Decrement the reference count on port while returning out of the loop.
> How did you find out about this issue?
I believe that Julia Lawall has been working on this for a while. After
doing some cleanup patches, I'm trying to continue the work with her help.
> I think it would be good to
> give credit to tool in commit log if the issue is identified or produced
> by tool. [In this case, I assume it's Coccinelle]
Yes, it was identified by Coccinelle and I should include it in the
commit message. Thank you for the remark. I'll revise the patch accordingly.
- Himadri
>> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
>> ---
>> net/dsa/dsa2.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index a1917025e155..396e7433dd8f 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -624,19 +624,25 @@ static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
>> for_each_available_child_of_node(ports, port) {
>> err = of_property_read_u32(port, "reg", ®);
>> if (err)
>> - return err;
>> + goto put_port;
>>
>> - if (reg >= ds->num_ports)
>> - return -EINVAL;
>> + if (reg >= ds->num_ports) {
>> + err = -EINVAL;
>> + goto put_port;
>> + }
>>
>> dp = &ds->ports[reg];
>>
>> err = dsa_port_parse_of(dp, port);
>> if (err)
>> - return err;
>> + goto put_port;
>> }
>>
>> return 0;
>> +
>> +put_port:
>> + of_node_put(port);
>> + return err;
>> }
>>
>> static int dsa_switch_parse_member_of(struct dsa_switch *ds,
>> --
>> 2.17.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190220032432.2878-1-himadri18.07%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [patch iproute2] devlink: relax dpipe table show dependency on resources
From: Jiri Pirko @ 2019-02-20 9:21 UTC (permalink / raw)
To: netdev; +Cc: stephen, dsahern, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Dpipe table show command has a depencency on getting resources.
If resource get command is not supported by the driver, dpipe table
show fails. However, resource is only additional information
in dpipe table show output. So relax the dependency and let
the dpipe tables be shown even if resources get command fails.
Fixes: ead180274caf ("devlink: Add support for resource/dpipe relation")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 3651e90c1159..00a76060b31d 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -4351,7 +4351,8 @@ static int dpipe_table_show(struct dpipe_ctx *ctx, struct nlattr *nl)
size = mnl_attr_get_u32(nla_table[DEVLINK_ATTR_DPIPE_TABLE_SIZE]);
counters_enabled = !!mnl_attr_get_u8(nla_table[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED]);
- resource_valid = !!nla_table[DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID];
+ resource_valid = !!nla_table[DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID] &&
+ ctx->resources;
if (resource_valid) {
table->resource_id = mnl_attr_get_u64(nla_table[DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID]);
table->resource_valid = true;
@@ -4467,12 +4468,9 @@ static int cmd_dpipe_table_show(struct dl *dl)
dl_opts_put(nlh, dl);
err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_resource_dump_cb,
&resource_ctx);
- if (err) {
- pr_err("error get resources %s\n", strerror(resource_ctx.err));
- goto err_resource_dump;
- }
+ if (!err)
+ dpipe_ctx.resources = resource_ctx.resources;
- dpipe_ctx.resources = resource_ctx.resources;
flags = NLM_F_REQUEST | NLM_F_ACK;
nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_DPIPE_TABLE_GET, flags);
dl_opts_put(nlh, dl);
@@ -4485,8 +4483,6 @@ static int cmd_dpipe_table_show(struct dl *dl)
dpipe_ctx_fini(&dpipe_ctx);
return 0;
-err_resource_dump:
- resource_ctx_fini(&resource_ctx);
err_resource_ctx_init:
err_headers_get:
dpipe_ctx_fini(&dpipe_ctx);
--
2.14.5
^ permalink raw reply related
* [PATCH][next] rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"
From: Colin King @ 2019-02-20 9:27 UTC (permalink / raw)
To: Ping-Ke Shih, Kalle Valo, David S . Miller, Larry Finger,
linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is an uppercase 'N' that should be a lowercase 'n', fix this.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
index 5cf5a85d1376..d8f496e2d68b 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
@@ -876,7 +876,7 @@ void rtl92ce_enable_hw_security_config(struct ieee80211_hw *hw)
u8 sec_reg_value;
RT_TRACE(rtlpriv, COMP_INIT, DBG_LOUD,
- "PairwiseENcAlgorithm = %d GroupEncAlgorithm = %d\n",
+ "PairwiseEncAlgorithm = %d GroupEncAlgorithm = %d\n",
rtlpriv->sec.pairwise_enc_algorithm,
rtlpriv->sec.group_enc_algorithm);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net] team: use operstate consistently for linkup
From: George Wilkie @ 2019-02-20 8:21 UTC (permalink / raw)
To: David Miller; +Cc: jiri, netdev
In-Reply-To: <20190219.141830.1399219758870863227.davem@davemloft.net>
On Tue, Feb 19, 2019 at 02:18:30PM -0800, David Miller wrote:
> From: George Wilkie <gwilkie@vyatta.att-mail.com>
> Date: Tue, 19 Feb 2019 15:57:15 +0000
>
> > Fixes: f1d22a1e0595 ("team: account for oper state")
> >
> > Signed-off-by: George Wilkie <gwilkie@vyatta.att-mail.com>
>
> Please do not put an empty line between Fixes: and other tags, all tags
> are equal and should be grouped together.
>
Fixed in v2. Thanks.
^ permalink raw reply
* BUG: assuming atomic context at kernel/seccomp.c:LINE
From: syzbot @ 2019-02-20 9:32 UTC (permalink / raw)
To: ast, daniel, kafai, keescook, linux-kernel, luto, netdev,
songliubraving, syzkaller-bugs, wad, yhs
Hello,
syzbot found the following crash on:
HEAD commit: abf446c90405 Add linux-next specific files for 20190220
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17f250d8c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
BUG: assuming atomic context at kernel/seccomp.c:271
in_atomic(): 0, irqs_disabled(): 0, pid: 12803, name: syz-executor.5
no locks held by syz-executor.5/12803.
CPU: 1 PID: 12803 Comm: syz-executor.5 Not tainted 5.0.0-rc7-next-20190220
#39
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
__cant_sleep kernel/sched/core.c:6218 [inline]
__cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
seccomp_run_filters kernel/seccomp.c:271 [inline]
__seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
__secure_computing+0x101/0x360 kernel/seccomp.c:932
syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45ac8a
Code: 25 18 00 00 00 00 74 01 f0 48 0f b1 3d df ba 5f 00 48 39 c2 75 da f3
c3 0f 1f 84 00 00 00 00 00 48 63 ff b8 e4 00 00 00 0f 05 <48> 3d 00 f0 ff
ff 77 06 f3 c3 0f 1f 40 00 48 c7 c2 d4 ff ff ff f7
RSP: 002b:00007f92ed7b2c58 EFLAGS: 00000246 ORIG_RAX: 00000000000000e4
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000045ac8a
RDX: 0000000000017230 RSI: 00007f92ed7b2c60 RDI: 0000000000000001
RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000004c4cd5 R14: 00000000004d8890 R15: 00000000ffffffff
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Luca Coelho @ 2019-02-20 9:39 UTC (permalink / raw)
To: Nick Desaulniers, Nathan Chancellor
Cc: Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
Kalle Valo, linux-wireless, netdev, LKML
In-Reply-To: <CAKwvOdmFE0kQoxvRDcarT+kYLXxqKYJfVHo4uTiYm7-wDefrrw@mail.gmail.com>
On Tue, 2019-02-19 at 11:05 -0800, Nick Desaulniers wrote:
> On Tue, Feb 19, 2019 at 10:21 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> > Clang warns:
> >
> > drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2:
> > warning:
> > comparison of distinct pointer types ('typeof ((rtt_avg)) *' (aka
> > 'long
> > long *') and 'uint64_t *' (aka 'unsigned long long *'))
> > [-Wcompare-distinct-pointer-types]
> > do_div(rtt_avg, 6666);
> > ^~~~~~~~~~~~~~~~~~~~~
> > include/asm-generic/div64.h:222:28: note: expanded from macro
> > 'do_div'
> > (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> > ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > do_div expects an unsigned dividend. Use div64_s64, which expects a
> > signed dividend.
>
> Eh, IIRC, signed vs unsigned division has implications for rounding
> towards zero or not, but I doubt that the round trip time average
> (RTT
> avg) should ever be negative. General rule of thumb for C is to keep
> arithmetic signed (even when working with non zero values), so rather
> than make the literal (6666) a unsigned long, I agree with your
> change
> to keep the division signed as well. Thanks for the fix.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for the patch and for the review. I've applied this to our
internal tree and it will be sent upstreaming following our normal
upstreaming process.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH net] sk_msg: Keep reference on socket file while psock lives
From: Jakub Sitnicki @ 2019-02-20 9:47 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, John Fastabend, Marek Majkowski
In-Reply-To: <5439765e-1288-c379-0ead-75597092a404@iogearbox.net>
Hi Daniel,
On Tue, Feb 19, 2019 at 05:00 PM CET, Daniel Borkmann wrote:
> On 02/11/2019 10:09 AM, Jakub Sitnicki wrote:
>> Backlog work for psock (sk_psock_backlog) might sleep while waiting for
>> memory to free up when sending packets. While sleeping, socket can
>> disappear from under our feet together with its wait queue because the
>> userspace has closed it.
>>
>> This breaks an assumption in sk_stream_wait_memory, which expects the
>> wait queue to be still there when it wakes up resulting in a
>> use-after-free:
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in remove_wait_queue+0x31/0x70
>> Write of size 8 at addr ffff888069a0c4e8 by task kworker/0:2/110
>>
>> CPU: 0 PID: 110 Comm: kworker/0:2 Not tainted 5.0.0-rc2-00335-g28f9d1a3d4fe-dirty #14
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014
>> Workqueue: events sk_psock_backlog
>> Call Trace:
>> print_address_description+0x6e/0x2b0
>> ? remove_wait_queue+0x31/0x70
>> kasan_report+0xfd/0x177
>> ? remove_wait_queue+0x31/0x70
>> ? remove_wait_queue+0x31/0x70
>> remove_wait_queue+0x31/0x70
>> sk_stream_wait_memory+0x4dd/0x5f0
>> ? sk_stream_wait_close+0x1b0/0x1b0
>> ? wait_woken+0xc0/0xc0
>> ? tcp_current_mss+0xc5/0x110
>> tcp_sendmsg_locked+0x634/0x15d0
>> ? tcp_set_state+0x2e0/0x2e0
>> ? __kasan_slab_free+0x1d1/0x230
>> ? kmem_cache_free+0x70/0x140
>> ? sk_psock_backlog+0x40c/0x4b0
>> ? process_one_work+0x40b/0x660
>> ? worker_thread+0x82/0x680
>> ? kthread+0x1b9/0x1e0
>> ? ret_from_fork+0x1f/0x30
>> ? check_preempt_curr+0xaf/0x130
>> ? iov_iter_kvec+0x5f/0x70
>> ? kernel_sendmsg_locked+0xa0/0xe0
>> skb_send_sock_locked+0x273/0x3c0
>> ? skb_splice_bits+0x180/0x180
>> ? start_thread+0xe0/0xe0
>> ? update_min_vruntime.constprop.27+0x88/0xc0
>> sk_psock_backlog+0xb3/0x4b0
>> ? strscpy+0xbf/0x1e0
>> process_one_work+0x40b/0x660
>> worker_thread+0x82/0x680
>> ? process_one_work+0x660/0x660
>> kthread+0x1b9/0x1e0
>> ? __kthread_create_on_node+0x250/0x250
>> ret_from_fork+0x1f/0x30
>>
>> Allocated by task 109:
>> sock_alloc_inode+0x54/0x120
>> alloc_inode+0x28/0xb0
>> new_inode_pseudo+0x7/0x60
>> sock_alloc+0x21/0xe0
>> __sys_accept4+0xc2/0x330
>> __x64_sys_accept+0x3b/0x50
>> do_syscall_64+0xb2/0x3e0
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> Freed by task 7:
>> kfree+0x7f/0x140
>> rcu_process_callbacks+0xe0/0x100
>> __do_softirq+0xe5/0x29a
>>
>> The buggy address belongs to the object at ffff888069a0c4e0
>> which belongs to the cache kmalloc-64 of size 64
>> The buggy address is located 8 bytes inside of
>> 64-byte region [ffff888069a0c4e0, ffff888069a0c520)
>> The buggy address belongs to the page:
>> page:ffffea0001a68300 count:1 mapcount:0 mapping:ffff88806d4018c0 index:0x0
>> flags: 0x4000000000000200(slab)
>> raw: 4000000000000200 dead000000000100 dead000000000200 ffff88806d4018c0
>> raw: 0000000000000000 00000000002a002a 00000001ffffffff 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888069a0c380: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
>> ffff888069a0c400: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>>> ffff888069a0c480: 00 00 00 00 00 00 00 00 fc fc fc fc fb fb fb fb
>> ^
>> ffff888069a0c500: fb fb fb fb fc fc fc fc fb fb fb fb fb fb fb fb
>> ffff888069a0c580: fc fc fc fc fb fb fb fb fb fb fb fb fc fc fc fc
>> ==================================================================
>>
>> Avoid it by keeping a reference to the socket file until the psock gets
>> destroyed.
>>
>> While at it, rearrange the order of reference grabbing and
>> initialization to match the destructor in reverse.
>>
>> Reported-by: Marek Majkowski <marek@cloudflare.com>
>> Link: https://lore.kernel.org/netdev/CAJPywTLwgXNEZ2dZVoa=udiZmtrWJ0q5SuBW64aYs0Y1khXX3A@mail.gmail.com
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> ---
>> net/core/skmsg.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
>> index 8c826603bf36..a38442b8580b 100644
>> --- a/net/core/skmsg.c
>> +++ b/net/core/skmsg.c
>> @@ -493,8 +493,13 @@ struct sk_psock *sk_psock_init(struct sock *sk, int node)
>> sk_psock_set_state(psock, SK_PSOCK_TX_ENABLED);
>> refcount_set(&psock->refcnt, 1);
>>
>> - rcu_assign_sk_user_data(sk, psock);
>> + /* Hold on to socket wait queue. Backlog work waits on it for
>> + * memory when sending. We must cancel work before socket wait
>> + * queue can go away.
>> + */
>> + get_file(sk->sk_socket->file);
>> sock_hold(sk);
>> + rcu_assign_sk_user_data(sk, psock);
>>
>> return psock;
>> }
>> @@ -558,6 +563,7 @@ static void sk_psock_destroy_deferred(struct work_struct *gc)
>> if (psock->sk_redir)
>> sock_put(psock->sk_redir);
>> sock_put(psock->sk);
>> + fput(psock->sk->sk_socket->file);
>
> Thanks for the report (and sorry for the late reply). I think holding ref on
> the struct file just so we keep it alive till deferred destruction might be
> papering over the actual underlying bug. Nothing obvious pops out from staring
> at the code right now; as a reproducer to run, did you use the prog in the link
> above and hit it after your strparser fix?
I get you, I actually sat on this fix for a moment because I had a
similar concern, that holding a file ref is a heavy-handed fix and I'm
not seeing the real problem.
For me it came down to this:
1. tcp_sendmsg_locked that we call from psock backlog work can end up
waiting for memory. We somehow need to ensure that the socket wait
queue does not disappear until tcp_sendmsg_locked returns.
2. KCM, which I assume must have the same problem, holds a reference on
the socket file.
I'm curious if there is another angle to it.
To answer your actual questions - your guesses are correct on both
accounts.
For the reproducer, I've used the TCP echo program from Marek [1]. On
the client side, I had something like:
while :; do
nc 10.0.0.1 12345 > /dev/null < /dev/zero &
pid=$!
sleep 0.1
kill $pid
done
I can dig out the test scripts or help testing any patches.
I was testing with the strparser fix applied, 1d79895aef18 ("sk_msg:
Always cancel strp work before freeing the psock"), which unfortunately
was not enough.
The explanation there was that the socket descriptor can get closed, and
in consequence the socket file can get destroyed, before the deferred
destructor for psock runs. So psock backlog work can be still very much
alive and running while the socket file is gone.
Thanks for looking into it,
-Jakub
[1] https://gist.github.com/majek/a09bcbeb8ab548cde6c18c930895c3f2
^ permalink raw reply
* Re: BUG: assuming atomic context at kernel/seccomp.c:LINE
From: Daniel Borkmann @ 2019-02-20 10:00 UTC (permalink / raw)
To: syzbot, ast, kafai, keescook, linux-kernel, luto, netdev,
songliubraving, syzkaller-bugs, wad, yhs
In-Reply-To: <000000000000cedfe1058250076c@google.com>
On 02/20/2019 10:32 AM, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: abf446c90405 Add linux-next specific files for 20190220
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17f250d8c00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
> dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
>
> BUG: assuming atomic context at kernel/seccomp.c:271
> in_atomic(): 0, irqs_disabled(): 0, pid: 12803, name: syz-executor.5
> no locks held by syz-executor.5/12803.
> CPU: 1 PID: 12803 Comm: syz-executor.5 Not tainted 5.0.0-rc7-next-20190220 #39
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> __cant_sleep kernel/sched/core.c:6218 [inline]
> __cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
> seccomp_run_filters kernel/seccomp.c:271 [inline]
> __seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
> __secure_computing+0x101/0x360 kernel/seccomp.c:932
> syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
> do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
False positive; bpf-next only. Pushing this out in a bit:
From d56547070162a105ff666f3324e558fa6492aedd Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 20 Feb 2019 10:51:17 +0100
Subject: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for
cbpf->ebpf progs
In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
a check was added for BPF_PROG_RUN() that for every invocation preemption is
disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
not count for seccomp because only cBPF -> eBPF is loaded here and it does not
make use of any functionality that would require this assertion. Fix this false
positive by adding and using __BPF_PROG_RUN() variant that does not have the
cant_sleep(); check.
Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/filter.h | 9 ++++++++-
kernel/seccomp.c | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f32b3ec..2f3e29a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -533,7 +533,14 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+#define bpf_prog_run__non_preempt(prog, ctx) \
+ ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
+/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
+#define BPF_PROG_RUN(prog, ctx) \
+ bpf_prog_run__non_preempt(prog, ctx)
+/* cBPF -> eBPF only, but not for native eBPF. */
+#define __BPF_PROG_RUN(prog, ctx) \
+ (*(prog)->bpf_func)(ctx, (prog)->insnsi)
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e815781..826d4e4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
* value always takes priority (ignoring the DATA).
*/
for (; f; f = f->prev) {
- u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
+ u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
ret = cur_ret;
--
2.9.5
^ permalink raw reply related
* Re: KASAN: use-after-free Read in br_mdb_ip_get
From: Herbert Xu @ 2019-02-20 10:23 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Nikolay Aleksandrov, Thomas Graf, syzbot, bridge, David Miller,
LKML, netdev, Roopa Prabhu, syzkaller-bugs
In-Reply-To: <CACT4Y+ZA8gBURbeZaDtrt5NoqFy8a8W3jyaWbs34Qjic4Bu+DA@mail.gmail.com>
On Mon, Jan 28, 2019 at 09:28:36AM +0100, Dmitry Vyukov wrote:
>
> > Weird, this is the kfree() on the error path of br_multicast_new_group()
> > when rhashtable_lookup_insert_fast() fails, which means the entry should
> > not be linked in the rhashtable or the hlist.
> > All other frees are via kfree_rcu. I'll keep looking..
>
> Humm.... +rhashtable.c maintianers
>
> The code in br_multicast_new_group is effectively:
>
> mp = kzalloc(sizeof(*mp), GFP_ATOMIC);
> err = rhashtable_lookup_insert_fast(&br->mdb_hash_tbl, &mp->rhnode,
> br_mdb_rht_params);
> if (err)
> kfree(mp);
>
> So it looks like rhashtable_lookup_insert_fast both returned an error
> and left the new object linked in the table. Since it happened only
> once, it may have something to do with concurrent resizing/shrinking.
I've looked through the rhashtable code in question and everything
looks OK. So I suspect some earlier corruption has occured to cause
this anomalous result. Is it possible to collect earlier alloc/free
stack traces on the object in question?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Russell King @ 2019-02-20 10:32 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Vivien Didelot; +Cc: David S. Miller, netdev
When a DSA port is added to a bridge and brought up, the resulting STP
state programmed into the hardware depends on the order that these
operations are performed. However, the Linux bridge code believes that
the port is in disabled mode.
If the DSA port is first added to a bridge and then brought up, it will
be in blocking mode. If it is brought up and then added to the bridge,
it will be in disabled mode.
This difference is caused by DSA always setting the STP mode in
dsa_port_enable() whether or not this port is part of a bridge. Since
bridge always sets the STP state when the port is added, brought up or
taken down, it is unnecessary for us to manipulate the STP state.
Apparently, this code was copied from Rocker, and the very next day a
similar fix for Rocker was merged but was not propagated to DSA. See
e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
net/dsa/port.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed0595459df1..ea7efc86b9d7 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -69,7 +69,6 @@ static void dsa_port_set_state_now(struct dsa_port *dp, u8 state)
int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
{
- u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
struct dsa_switch *ds = dp->ds;
int port = dp->index;
int err;
@@ -80,7 +79,8 @@ int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
return err;
}
- dsa_port_set_state_now(dp, stp_state);
+ if (!dp->bridge_dev)
+ dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
return 0;
}
@@ -90,7 +90,8 @@ void dsa_port_disable(struct dsa_port *dp, struct phy_device *phy)
struct dsa_switch *ds = dp->ds;
int port = dp->index;
- dsa_port_set_state_now(dp, BR_STATE_DISABLED);
+ if (!dp->bridge_dev)
+ dsa_port_set_state_now(dp, BR_STATE_DISABLED);
if (ds->ops->port_disable)
ds->ops->port_disable(ds, port, phy);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Arnd Bergmann @ 2019-02-20 10:51 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, linux-wireless, Networking,
Linux Kernel Mailing List, Nick Desaulniers
In-Reply-To: <20190219182105.19933-1-natechancellor@gmail.com>
On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> index e9822a3ec373..92b22250eb7d 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct iwl_mvm *mvm, u8 index,
> {
> s64 rtt_avg = res->ftm.rtt_avg * 100;
>
> - do_div(rtt_avg, 6666);
> + div64_s64(rtt_avg, 6666);
This is wrong: div64_s64 does not modify its argument like do_div(), but
it returns the result instead. You also don't want to divide by a 64-bit
value when the second argument is a small constant.
I think the correct way should be
s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
If you know that the value is positive, using unsigned types
and div_u64() would be slightly faster.
Arnd
^ permalink raw reply
* Re: [PATCH net-next v2 07/10] net: phy: marvell10g: Add support for 2.5GBASET
From: Maxime Chevallier @ 2019-02-20 10:54 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: davem, netdev, linux-kernel, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-arm-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
mw
In-Reply-To: <20190207234824.dhynfaglfxllxhei@shell.armlinux.org.uk>
Hello Russell,
On Thu, 7 Feb 2019 23:48:24 +0000
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>On Thu, Feb 07, 2019 at 10:49:36AM +0100, Maxime Chevallier wrote:
>> The Marvell Alaska family of PHYs supports 2.5GBaseT and 5GBaseT modes,
>> as defined in the 802.3bz specification.
>>
>> When the link partner requests a 2.5GBASET link, the PHY will
>> reconfigure it's MII interface to 2500BASEX.
>>
>> At 5G, the PHY will reconfigure it's interface to 5GBASE-R, but this
>> mode isn't supported by any MAC for now.
>>
>> This was tested with :
>> - The 88X3310, which is on the MacchiatoBin
>
>Hi Maxime,
>
>Looking deeper at this, I think we actually need an additional patch at
>the beginning of your series.
>
>The default AN advertisement in 7.32 is 0x1181 - which includes the
>2.5G and 5G modes. We need to clear these bits, so that when the 10G
>mode disabled via ethtool, we do not switch to 2.5G or 5G speed (both
>of which are not currently reported as supported.) Such a patch needs
>backporting to stable kernels.
Good catch. The issue seems fixed by Andrew's patch :
3de97f3c6308 ("net: phy: marvell10g: use genphy_c45_an_config_aneg")
However, the fix should indeed be backported to the -stable trees, I've
been able to repdocude this on 4.20. I'll take care of sending a patch
to -net for that.
Thanks for reporting this,
Maxime
^ permalink raw reply
* [PATCH v3 4/8] net: thunderx: add nicvf_send_msg_to_pf result check for set_rx_mode_task
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
The rx_set_mode invokes number of messages to be send to PF for receive
mode configuration. In case if there any issues we need to stop sending
messages and release allocated memory.
This commit is to implement check of nicvf_msg_send_to_pf() result.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 19b58fc3ca41..45f06504a61b 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1953,7 +1953,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
/* flush DMAC filters and reset RX mode */
mbx.xcast.msg = NIC_MBOX_MSG_RESET_XCAST;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
if (mode & BGX_XCAST_MCAST_FILTER) {
/* once enabling filtering, we need to signal to PF to add
@@ -1961,7 +1962,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
*/
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = 0;
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
/* check if we have any specific MACs to be added to PF DMAC filter */
@@ -1970,9 +1972,9 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
for (idx = 0; idx < mc_addrs->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
mbx.xcast.data.mac = mc_addrs->mc[idx];
- nicvf_send_msg_to_pf(nic, &mbx);
+ if (nicvf_send_msg_to_pf(nic, &mbx) < 0)
+ goto free_mc;
}
- kfree(mc_addrs);
}
/* and finally set rx mode for PF accordingly */
@@ -1980,6 +1982,8 @@ static void __nicvf_set_rx_mode_task(u8 mode, struct xcast_addr_list *mc_addrs,
mbx.xcast.data.mode = mode;
nicvf_send_msg_to_pf(nic, &mbx);
+free_mc:
+ kfree(mc_addrs);
}
static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
--
2.17.2
^ permalink raw reply related
* [PATCH v3 6/8] net: thunderx: add mutex to protect mailbox from concurrent calls for same VF
From: Vadim Lomovtsev @ 2019-02-20 11:02 UTC (permalink / raw)
To: sgoutham@cavium.com, sunil.kovvuri@gmail.com, rric@kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net
Cc: dnelson@redhat.com, Vadim Lomovtsev
In-Reply-To: <20190220110225.9497-1-vlomovtsev@marvell.com>
In some cases it could happen that nicvf_send_msg_to_pf() could be called
concurrently for the same NIC VF, and thus re-writing mailbox contents and
breaking messaging sequence with PF by re-writing NICVF data.
This commit is to implement mutex for NICVF to protect mailbox registers
and NICVF messaging control data from concurrent access.
Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 2 ++
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 13 ++++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 227343625e83..86cda3f4b37b 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -329,6 +329,8 @@ struct nicvf {
spinlock_t rx_mode_wq_lock;
/* workqueue for handling kernel ndo_set_rx_mode() calls */
struct workqueue_struct *nicvf_rx_mode_wq;
+ /* mutex to protect VF's mailbox contents from concurrent access */
+ struct mutex rx_mode_mtx;
/* PTP timestamp */
struct cavium_ptp *ptp_clock;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index da5986ca7bee..2332e3e95e0e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -124,6 +124,9 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
{
int timeout = NIC_MBOX_MSG_TIMEOUT;
int sleep = 10;
+ int ret = 0;
+
+ mutex_lock(&nic->rx_mode_mtx);
nic->pf_acked = false;
nic->pf_nacked = false;
@@ -136,7 +139,8 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF NACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}
msleep(sleep);
if (nic->pf_acked)
@@ -146,10 +150,12 @@ int nicvf_send_msg_to_pf(struct nicvf *nic, union nic_mbx *mbx)
netdev_err(nic->netdev,
"PF didn't ACK to mbox msg 0x%02x from VF%d\n",
(mbx->msg.msg & 0xFF), nic->vf_id);
- return -EBUSY;
+ ret = -EBUSY;
+ break;
}
}
- return 0;
+ mutex_unlock(&nic->rx_mode_mtx);
+ return ret;
}
/* Checks if VF is able to comminicate with PF
@@ -2208,6 +2214,7 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
nic->vf_id);
INIT_WORK(&nic->rx_mode_work.work, nicvf_set_rx_mode_task);
spin_lock_init(&nic->rx_mode_wq_lock);
+ mutex_init(&nic->rx_mode_mtx);
err = register_netdev(netdev);
if (err) {
--
2.17.2
^ 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