* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: David Ahern @ 2019-08-27 13:43 UTC (permalink / raw)
To: Michal Kubecek, netdev
Cc: Jakub Kicinski, Jiri Pirko, Roopa Prabhu, David Miller,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
In-Reply-To: <20190827045542.GE29594@unicorn.suse.cz>
On 8/26/19 10:55 PM, Michal Kubecek wrote:
> On Mon, Aug 26, 2019 at 03:46:43PM -0600, David Ahern wrote:
>> On 8/26/19 10:55 AM, Jakub Kicinski wrote:
>>> On Mon, 26 Aug 2019 18:09:16 +0200, Jiri Pirko wrote:
>>>> DaveA, Roopa. Do you insist on doing add/remove of altnames in the
>>>> existing setlist command using embedded message op attrs? I'm asking
>>>> because after some time thinking about it, it still feels wrong to me :/
>>>>
>>>> If this would be a generic netlink api, we would just add another couple
>>>> of commands. What is so different we can't add commands here?
>>>> It is also much simpler code. Easy error handling, no need for
>>>> rollback, no possibly inconsistent state, etc.
>>>
>>> +1 the separate op feels like a better uapi to me as well.
>>>
>>> Perhaps we could redo the iproute2 command line interface to make the
>>> name the primary object? Would that address your concern Dave and Roopa?
>>>
>>
>> No, my point is exactly that a name is not a primary object. A name is
>> an attribute of a link - something that exists for the convenience of
>> userspace only. (Like the 'protocol' for routes, rules and neighbors.)
>>
>> Currently, names are changed by RTM_NEWLINK/RTM_SETLINK. Aliases are
>> added and deleted by RTM_NEWLINK/RTM_SETLINK. Why is an alternative name
>> so special that it should have its own API?
>
> There is only one alias so that it makes perfect sense to set it like
> any other attribute. But the series introduces a list of alternative
> names. So IMHO better analogy would be network addresses - and we do
> have RTM_NEWADDR/RTM_DELADDR for them.
RTM_*ADDR manage network layer addresses. Those are anchored to a device
but not direct attributes describing the device.
The device names are just alternative (human friendly) references to a
specific device hence they should be direct link attributes.
^ permalink raw reply
* Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-27 13:35 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866792BEAAB1958BB5A9C4AD1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:57:07 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 5:11 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> >
> > On Tue, 27 Aug 2019 11:33:54 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 4:54 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > > >
> > > > On Tue, 27 Aug 2019 11:12:23 +0000
> > > > Parav Pandit <parav@mellanox.com> wrote:
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > > > Sent: Tuesday, August 27, 2019 3:54 PM
> > > > > > To: Parav Pandit <parav@mellanox.com>
> > > > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > > > > >
> >
> > > > > > What about:
> > > > > >
> > > > > > * @get_alias_length: optional callback to specify length of the
> > > > > > alias to
> > > > create
> > > > > > * Returns unsigned integer: length of the alias to be created,
> > > > > > * 0 to not create an alias
> > > > > >
> > > > > Ack.
> > > > >
> > > > > > I also think it might be beneficial to add a device parameter
> > > > > > here now (rather than later); that seems to be something that makes
> > sense.
> > > > > >
> > > > > Without showing the use, it shouldn't be added.
> > > >
> > > > It just feels like an omission: Why should the vendor driver only be
> > > > able to return one value here, without knowing which device it is for?
> > > > If a driver supports different devices, it may have different
> > > > requirements for them.
> > > >
> > > Sure. Lets first have this requirement to add it.
> > > I am against adding this length field itself without an actual vendor use case,
> > which is adding some complexity in code today.
> > > But it was ok to have length field instead of bool.
> > >
> > > Lets not further add "no-requirement futuristic knobs" which hasn't shown its
> > need yet.
> > > When a vendor driver needs it, there is nothing prevents such addition.
> >
> > Frankly, I do not see how it adds complexity; the other callbacks have device
> > arguments already,
> Other ioctls such as create, remove, mmap, likely need to access the parent.
> Hence it make sense to have parent pointer in there.
>
> I am not against complexity, I am just saying, at present there is no use-case. Let have use case and we add it.
>
> > and the vendor driver is free to ignore it if it does not have
> > a use for it. I'd rather add the argument before a possible future user tries
> > weird hacks to allow multiple values, but I'll leave the decision to the
> > maintainers.
> Why would a possible future user tries a weird hack?
> If user needs to access parent device, that driver maintainer should ask for it.
I've seen the situation often enough that folks tried to do hacks
instead of enhancing the interface.
Again, let's get a maintainer opinion.
^ permalink raw reply
* Re: [PATCH 0/4] Introduce variable length mdev alias
From: Cornelia Huck @ 2019-08-27 13:31 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866A24FF3D283F0F3CB3CDAD1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 13:11:17 +0000
Parav Pandit <parav@mellanox.com> wrote:
> Hi Alex, Cornelia,
>
> > -----Original Message-----
> > From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> > Of Parav Pandit
> > Sent: Tuesday, August 27, 2019 2:11 AM
> > To: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net
> > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; Parav Pandit <parav@mellanox.com>
> > Subject: [PATCH 0/4] Introduce variable length mdev alias
> >
> > To have consistent naming for the netdevice of a mdev and to have consistent
> > naming of the devlink port [1] of a mdev, which is formed using
> > phys_port_name of the devlink port, current UUID is not usable because UUID
> > is too long.
> >
> > UUID in string format is 36-characters long and in binary 128-bit.
> > Both formats are not able to fit within 15 characters limit of netdev name.
> >
> > It is desired to have mdev device naming consistent using UUID.
> > So that widely used user space framework such as ovs [2] can make use of
> > mdev representor in similar way as PCIe SR-IOV VF and PF representors.
> >
> > Hence,
> > (a) mdev alias is created which is derived using sha1 from the mdev name.
> > (b) Vendor driver describes how long an alias should be for the child mdev
> > created for a given parent.
> > (c) Mdev aliases are unique at system level.
> > (d) alias is created optionally whenever parent requested.
> > This ensures that non networking mdev parents can function without alias
> > creation overhead.
> >
> > This design is discussed at [3].
> >
> > An example systemd/udev extension will have,
> >
> > 1. netdev name created using mdev alias available in sysfs.
> >
> > mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> > mdev 12 character alias=cd5b146a80a5
> >
> > netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link m =
> > mediated device
> >
> > 2. devlink port phys_port_name created using mdev alias.
> > devlink phys_port_name=pcd5b146a80a5
> >
> > This patchset enables mdev core to maintain unique alias for a mdev.
> >
> > Patch-1 Introduces mdev alias using sha1.
> > Patch-2 Ensures that mdev alias is unique in a system.
> > Patch-3 Exposes mdev alias in a sysfs hirerchy.
> > Patch-4 Extends mtty driver to optionally provide alias generation.
> > This also enables to test UUID based sha1 collision and trigger error handling
> > for duplicate sha1 results.
> >
> > In future when networking driver wants to use mdev alias, mdev_alias() API will
> > be added to derive devlink port name.
> >
> Now that majority of above patches looks in shape and I addressed all comments,
I think the discussion of what to do with the attribute if no alias is
available is still unresolved; waiting for maintainer opinion.
> In next v1 post, I was considering to include mdev_alias() and have example use in mtty driver.
>
> This way, subsequent series of mlx5_core who intents to use mdev_alias() API makes it easy to review and merge through Dave M, netdev tree.
> Is that ok with you?
^ permalink raw reply
* Re: [PATCH 3/4] net: dsa: microchip: fix interrupt mask
From: Razvan.Stefanescu @ 2019-08-27 13:22 UTC (permalink / raw)
To: andrew
Cc: Woojung.Huh, UNGLinuxDriver, vivien.didelot, f.fainelli, davem,
netdev, linux-kernel
In-Reply-To: <20190827125135.GA11471@lunn.ch>
On 27/08/2019 15:51, Andrew Lunn wrote:
>
> On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
>> Global Interrupt Mask Register comprises of Lookup Engine (LUE) Interrupt
>> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
>> Mask (bit 29).
>>
>> This corrects LUE bit.
>
> Hi Razvan
>
> Is this a fix? Something that should be back ported to old kernels?
Hello,
During testing I did not observed any issues with the old value. So I am
not sure how the switch is affected by the incorrect setting.
Maybe maintainers will be able to make a better assessment if this needs
back-porting. And I will be happy to do it if it is necessary.
Best regards,
Razvan
^ permalink raw reply
* Re: [PATCH v2 0/3] Add NETIF_F_HW_BR_CAP feature
From: Andrew Lunn @ 2019-08-27 13:18 UTC (permalink / raw)
To: Horatiu Vultur
Cc: roopa, nikolay, davem, UNGLinuxDriver, alexandre.belloni,
allan.nielsen, f.fainelli, netdev, linux-kernel, bridge
In-Reply-To: <20190827101033.g2cb6j2j4kuyzh2a@soft-dev3.microsemi.net>
> That sounds like a great idea. I was expecting to add this logic in the
> set_rx_mode function of the driver. But unfortunetly, I got the calls to
> this function before the dev->promiscuity is updated or not to get the
> call at all. For example in case the port is member of a bridge and I try
> to enable the promisc mode.
Hi Horatiu
What about the notifier? Is it called in all the conditions you need
to know about?
Or, you could consider adding a new switchdev call to pass this
information to any switchdev driver which is interested in the
information.
At the moment, the DSA driver core does not pass onto the driver it
should put a port into promisc mode. So pcap etc, will only see
traffic directed to the CPU, not all the traffic ingressing the
interface. If you put the needed core infrastructure into place, we
could plumb it down from the DSA core to DSA drivers.
Having said that, i don't actually know if the Marvell switches
support this. Forward using the ATU and send a copy to the CPU? What
switches tend to support is port mirroring, sending all the traffic
out another port. A couple of DSA drivers support that, via TC.
Andrew
^ permalink raw reply
* RE: [PATCH 0/4] Introduce variable length mdev alias
From: Parav Pandit @ 2019-08-27 13:11 UTC (permalink / raw)
To: Parav Pandit, alex.williamson@redhat.com, Jiri Pirko,
kwankhede@nvidia.com, cohuck@redhat.com, davem@davemloft.net
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20190826204119.54386-1-parav@mellanox.com>
Hi Alex, Cornelia,
> -----Original Message-----
> From: kvm-owner@vger.kernel.org <kvm-owner@vger.kernel.org> On Behalf
> Of Parav Pandit
> Sent: Tuesday, August 27, 2019 2:11 AM
> To: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net
> Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; Parav Pandit <parav@mellanox.com>
> Subject: [PATCH 0/4] Introduce variable length mdev alias
>
> To have consistent naming for the netdevice of a mdev and to have consistent
> naming of the devlink port [1] of a mdev, which is formed using
> phys_port_name of the devlink port, current UUID is not usable because UUID
> is too long.
>
> UUID in string format is 36-characters long and in binary 128-bit.
> Both formats are not able to fit within 15 characters limit of netdev name.
>
> It is desired to have mdev device naming consistent using UUID.
> So that widely used user space framework such as ovs [2] can make use of
> mdev representor in similar way as PCIe SR-IOV VF and PF representors.
>
> Hence,
> (a) mdev alias is created which is derived using sha1 from the mdev name.
> (b) Vendor driver describes how long an alias should be for the child mdev
> created for a given parent.
> (c) Mdev aliases are unique at system level.
> (d) alias is created optionally whenever parent requested.
> This ensures that non networking mdev parents can function without alias
> creation overhead.
>
> This design is discussed at [3].
>
> An example systemd/udev extension will have,
>
> 1. netdev name created using mdev alias available in sysfs.
>
> mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> mdev 12 character alias=cd5b146a80a5
>
> netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link m =
> mediated device
>
> 2. devlink port phys_port_name created using mdev alias.
> devlink phys_port_name=pcd5b146a80a5
>
> This patchset enables mdev core to maintain unique alias for a mdev.
>
> Patch-1 Introduces mdev alias using sha1.
> Patch-2 Ensures that mdev alias is unique in a system.
> Patch-3 Exposes mdev alias in a sysfs hirerchy.
> Patch-4 Extends mtty driver to optionally provide alias generation.
> This also enables to test UUID based sha1 collision and trigger error handling
> for duplicate sha1 results.
>
> In future when networking driver wants to use mdev alias, mdev_alias() API will
> be added to derive devlink port name.
>
Now that majority of above patches looks in shape and I addressed all comments,
In next v1 post, I was considering to include mdev_alias() and have example use in mtty driver.
This way, subsequent series of mlx5_core who intents to use mdev_alias() API makes it easy to review and merge through Dave M, netdev tree.
Is that ok with you?
^ permalink raw reply
* Re: [PATCH net-next] drop_monitor: Make timestamps y2038 safe
From: Arnd Bergmann @ 2019-08-27 13:10 UTC (permalink / raw)
To: Ido Schimmel
Cc: Networking, David Miller, Neil Horman, Andrew Lunn, Aya Levin,
mlxsw, Ido Schimmel
In-Reply-To: <20190823154721.9927-1-idosch@idosch.org>
On Fri, Aug 23, 2019 at 5:48 PM Ido Schimmel <idosch@idosch.org> wrote:
>
> From: Ido Schimmel <idosch@mellanox.com>
>
> Timestamps are currently communicated to user space as 'struct
> timespec', which is not considered y2038 safe since it uses a 32-bit
> signed value for seconds.
>
> Fix this while the API is still not part of any official kernel release
> by using 64-bit nanoseconds timestamps instead.
>
> Fixes: ca30707dee2b ("drop_monitor: Add packet alert mode")
> Fixes: 5e58109b1ea4 ("drop_monitor: Add support for packet alert mode for hardware drops")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
> Arnd, I have followed your recommendation to use 64-bit nanoseconds
> timestamps. I would appreciate it if you could review this change.
somewhat late reply:
> @@ -761,8 +759,8 @@ static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
> goto nla_put_failure;
> }
>
> - if (ktime_to_timespec_cond(skb->tstamp, &ts) &&
> - nla_put(msg, NET_DM_ATTR_TIMESTAMP, sizeof(ts), &ts))
> + if (nla_put_u64_64bit(msg, NET_DM_ATTR_TIMESTAMP,
> + ktime_to_ns(skb->tstamp), NET_DM_ATTR_PAD))
> goto nla_put_failure;
>
> if (nla_put_u32(msg, NET_DM_ATTR_ORIG_LEN, skb->len))
Yes, this looks reasonable. I guess since we take the skb timestamps
in CLOCK_REALTIME, there is little point in trying to use
CLOCK_MONOTONIC in the user interface.
64-bit nanoseconds are safe for a few hundred years.
Arnd
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix lack of conn cleanup when local endpoint is cleaned up
From: David Howells @ 2019-08-27 12:57 UTC (permalink / raw)
To: David Miller; +Cc: dhowells, netdev, marc.dionne, linux-afs, linux-kernel
In-Reply-To: <20190824.143533.1547411490171696760.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
> Once you've removed the entries from the globally visible idle_client_comms
> list, and put them on the local garbage list, they cannot be seen in any way
> by external threads of control outside of this function.
Yeah, I think you're right. I was thinking that it might race with
rxrpc_discard_expired_client_conns() but that takes the locks too, so it
shouldn't.
David
^ permalink raw reply
* Re: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count
From: Andrew Lunn @ 2019-08-27 12:55 UTC (permalink / raw)
To: Razvan Stefanescu
Cc: Woojung Huh, Microchip Linux Driver Support, Vivien Didelot,
Florian Fainelli, David S . Miller, netdev, linux-kernel
In-Reply-To: <20190827093110.14957-5-razvan.stefanescu@microchip.com>
On Tue, Aug 27, 2019 at 12:31:10PM +0300, Razvan Stefanescu wrote:
> Use port_cnt value to disable interrupts on switch reset.
>
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
> ---
> drivers/net/dsa/microchip/ksz9477.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 187be42de5f1..54fc05595d48 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -213,7 +213,7 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
>
> /* disable interrupts */
> ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
> - ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
> + ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev->port_cnt);
> ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
Humm, is this correct? 0x7f suggests this is a bitmap for 7 ports.
The name port_cnt suggests it is a count, e.g. 7 for 7 ports.
0x7f != 7.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH 3/4] net: dsa: microchip: fix interrupt mask
From: Andrew Lunn @ 2019-08-27 12:51 UTC (permalink / raw)
To: Razvan Stefanescu
Cc: Woojung Huh, Microchip Linux Driver Support, Vivien Didelot,
Florian Fainelli, David S . Miller, netdev, linux-kernel
In-Reply-To: <20190827093110.14957-4-razvan.stefanescu@microchip.com>
On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
> Global Interrupt Mask Register comprises of Lookup Engine (LUE) Interrupt
> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
> Mask (bit 29).
>
> This corrects LUE bit.
Hi Razvan
Is this a fix? Something that should be back ported to old kernels?
Thanks
Andrew
^ permalink raw reply
* Re: BUG_ON in skb_segment, after bpf_skb_change_proto was applied
From: Daniel Borkmann @ 2019-08-27 12:10 UTC (permalink / raw)
To: Shmulik Ladkani, Eric Dumazet
Cc: netdev, Alexander Duyck, Alexei Starovoitov, Yonghong Song,
Steffen Klassert, shmulik, eyal
In-Reply-To: <20190827144218.5b098eac@pixies>
On 8/27/19 1:42 PM, Shmulik Ladkani wrote:
[...]
> - Another thing that puzzles me is that we hit the BUG_ON rather rarely
> and cannot yet reproduce synthetically. If skb_segment's handling of
> skbs with a frag_list (that have gso_size mangled) is broken, I'd expect
> to hit this more often... Any ideas?
>
> - Suppose going for a rewrite, care to elaborate what's exactly missing
> in skb_segment's logic?
> I must admit I do not fully understand all the different code flows in
> this function, it seems to support many different input skbs - any
> assistance is highly appreciated.
Given first point above wrt hitting rarely, it would be good to first get a
better understanding for writing a reproducer. Back then Yonghong added one
to the BPF kernel test suite [0], so it would be desirable to extend it for
the case you're hitting. Given NAT64 use-case is needed and used by multiple
parties, we should try to (fully) fix it generically.
Thanks,
Daniel
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76db8087c4c991dcd17f5ea8ac0eafd0696ab450
> Shmulik
>
^ permalink raw reply
* Re: [PATCH] rtl_nic: add firmware rtl8125a-3
From: Josh Boyer @ 2019-08-27 12:08 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Linux Firmware, Chun-Hao Lin, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <dd3220fb-7ed3-c5d1-d501-5e94f270a6b4@gmail.com>
On Mon, Aug 26, 2019 at 6:23 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> This adds firmware rtl8125a-3 for Realtek's 2.5Gbps chip RTL8125.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> Firmware file was provided by Realtek and they asked me to submit it.
Can we get a Signed-off-by from someone at Realtek then?
josh
> The related extension to r8169 driver will be submitted in the next days.
> ---
> WHENCE | 3 +++
> rtl_nic/rtl8125a-3.fw | Bin 0 -> 3456 bytes
> 2 files changed, 3 insertions(+)
> create mode 100644 rtl_nic/rtl8125a-3.fw
>
> diff --git a/WHENCE b/WHENCE
> index fb12924..dbec18a 100644
> --- a/WHENCE
> +++ b/WHENCE
> @@ -2906,6 +2906,9 @@ Version: 0.0.2
> File: rtl_nic/rtl8107e-2.fw
> Version: 0.0.2
>
> +File: rtl_nic/rtl8125a-3.fw
> +Version: 0.0.1
> +
> Licence:
> * Copyright © 2011-2013, Realtek Semiconductor Corporation
> *
> diff --git a/rtl_nic/rtl8125a-3.fw b/rtl_nic/rtl8125a-3.fw
> new file mode 100644
> index 0000000000000000000000000000000000000000..fac635263f92e8d9734456b75932b2088edd5ef9
> GIT binary patch
> literal 3456
> zcmb7G4@{Kj8Gr9M&hw<l39l3>p*MPE#webM6qsqQiuC(hXPi?+wDL#V(Z*4#K%FD{
> zd#PH+tTJbqZG?a`)*5G*m3F2zmN`7hx;2*IKiVu;;~Hwa_E@^5+Z^ooav$qyWa|!|
> z{J!V^^FHtMe%~vE5S!~a<<HMqSUGn=c_2HGJ>M6|pO=$6Z+-!F`d2|JjuT=?tX!6t
> zo1eG1ysol-V@-KgU0(T?#@f6Efr9d!!2E*zz=G_mCu@CyK;goi!iD+b1yk6B2%b&6
> z7edS;xkzqO0?9-2l9EW0ltM}+rIFG}86+PmljJ95fhB~6Z~~0y3JYX}?Z^&0uqy1t
> zny?FN!)}}nC*Ym12kkF<@t&E4_=v=ynF9AnDz2DmaE+uRv6r!*@!^m!6NmhMOzq8r
> zcySgS;n{HZ&ViVjO+Em7C<o$9E{w7~#?6OA6vF-CA|!?$MBkPmUNaZNIZ}kPTZ-Wd
> z8F2PIf~lcpz}RxchgOhZNnAy~1V!<ssEIFw^W*gx{=)|N&sV@7s=~F-YS=QKK)AC8
> z;l`&BHaB5(q!u4F*5UicX4Dw<xZc@_xQwl|*+!ct+H9u{FeB7V|DE*TO<fCht<>$I
> zZZG}Y@U*d?z6a>rPW?gZU!wjH^_^&crVIA-hauiRf}*mc@O*L%b>cXFD^8&Io|9br
> zFS+(#A<NUl=QsF#3U7Megl)!Y%#NIan9+;JM$V!#)QA3t5H6bia7TWJOXlz4ioA=<
> z<^?z-1MK-A9Fa>HXt;u_<`7nle1Pws`y;xZ4b$%$RvXt*Vtj-(#xP2a8yGS^#rwu*
> z7&RlXNB)8`;|q+Lf8+C)SZDkL{T(+MYZPk@p$0naYDvhUdK;W-&~&K<5x2TvCa4ES
> zJSq_Os=`o`>Ti(hqM4#RkyLfbOj8MwbamOxQ0|CNT`@D2E8<rJ4O!}{IZMSyW~=Pb
> z9LCG0O+ej0lA~sw%T-;^=BaOn@)@g8tu_{^65~O&#t5oXW3d`Ciq!i?u^KfEWsf|f
> z%8X@d%v{dr6>6QaQuTMNV*C=d)+lAYWhy1Kp7A%(ze4qPRH@`pHTfshkXfVRB2TgY
> zO=`+Wt(q39Q=W61)m5XOc8&DkO5CPgp(bTNzg>y9p~8jDs$zJj5<R=s;Oi~Q^>M4(
> zG`vTNq`hitUz@`B_Nx)&fWp3Z<>))8?4g&GICDrH_jW38Z<lJiahS3rlpR$<98=@x
> z6ix)=-SB%nIOXwTePI%gc_p@upI;Gdo~F;Tmw)5`y%^_39nX1Ki-J6~L1FKz7NSjv
> z_`nkPz3?L$w%rohr-(vgBF1<i;qBEnDP75AC6X+Z8dD{_;JbnNV+;3L_)`miW?_eg
> z-4-4vre2o#VTHs{P{PCaz|dle45F5=t7KfRt3*ceh=r$#TAy7mkxtwHW#j)EHgmp)
> zO)hO_GH>cb5{`=!(@#j)h_-jxgCneiD9Hcd;ix`Q>rX~yy2c{beS3_|m>9m87;BeK
> z9^<~->L`kd5sZmZuw?QWNw>vliHU)j7&EQ4-f1m1#+c~6-j5U9k9~bn**#pV?$N|-
> zq$Nr8T#$I{J?c3t2VJ-F9=Aj_^{tkE`!wz?XF~Vaag5Xw_4`&lQTP1kQQmM$_|B0(
> zOkxdl@0Tb}k#O<ZNq?P7^BP@o5?P$tDMUYUDdm~Ohjk2MA!9p<P0Z~eCa@+uv7NOF
> zV)t~$Ac`@GiSytUlb?qfh~`cEKhXXQ`gkQS+oQgJX8gniiK%+szlqBBQMQ+LjIoYA
> z7Pea0V&QHJcUss?e1U!-enM;`#@W7FhmW$!&b34|Z>oiU3%_IGY6~}8_<=_875$5K
> z-p<>elW^0nO-Xb$v)?-<5?*G%4-kJO5#)Zu+T&X8&rjZA4DRX1phN*@#Ln3Z5`&x>
> zM^Cgz;x5{-ci}0VyQ8FT#^zi&IL{9H?xHU!(>!!e${wT4$GqBa3H>xiG*Va3d7hBl
> z$-lAV)_T3WsTa}QwwT;{)^yFKtnb^bPxFJExzl=W&oejIewS6dj^AhH>tVh&*5~e`
> zja9dg7?{KsZ_$^!dgjpQL9gf03g&nvnzMv6vp=S9DYVsnn`y<1qkR|cAF7hLwo&4_
> z$0b%#ug_6NWfb*$Il}q#a(!Ob6=ePMXrpB-v<#HJEP3C!v!5@<?;Dn1MU0<*f8Qxz
> zAjU7*@~eyS)8C3a`2}PA^u1Eoi5NfK?_u^^&&MtM&ozI+{=2wFEidM}?R=ite~tg7
> zpYGYyoP*`0xugV=o@1RyFwcMDQ>Obe;jicCb=vBh2MhU4<KB$Vk2NcQ<(`-qXixrA
> z8^49!$+$sGAily`auZvb-$fkYEIElgD0dJaC)$bQXUsw`@urBL?<RiJ1G^-)6L-BT
> z@#-RprR2w-mqwqPKV$B{bA8MiN1HEyEpap~@gZZ_*el!TTw^JF*(K5a0Q2#@rtsUg
> zt6SovowDhaJ<ob6YnhO-UOUgSow9UNkB-M#+sN~3dy@Qhi9cEVf73Z`N^D^5uW{WK
> zME%~Yvas61t;E=S%Z@SO6V|;&h-h!3cbdD!=(z6g@jH#a_vpS&+;={={DQpi2<!K6
> DeEY6F
>
> literal 0
> HcmV?d00001
>
> --
> 2.23.0
>
^ permalink raw reply
* RE: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
From: Parav Pandit @ 2019-08-27 12:00 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827135527.7c9d3940.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, August 27, 2019 5:25 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
>
> On Tue, 27 Aug 2019 11:52:21 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 5:05 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> > >
> > > On Tue, 27 Aug 2019 11:07:37 +0000
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > > Sent: Tuesday, August 27, 2019 4:17 PM
> > > > > To: Parav Pandit <parav@mellanox.com>
> > > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> > > > >
> > > > > On Mon, 26 Aug 2019 15:41:18 -0500 Parav Pandit
> > > > > <parav@mellanox.com> wrote:
> > >
> > > > > > +static ssize_t alias_show(struct device *device,
> > > > > > + struct device_attribute *attr, char *buf) {
> > > > > > + struct mdev_device *dev = mdev_from_dev(device);
> > > > > > +
> > > > > > + if (!dev->alias)
> > > > > > + return -EOPNOTSUPP;
> > > > >
> > > > > I'm wondering how to make this consumable by userspace in the
> > > > > easiest
> > > way.
> > > > > - As you do now (userspace gets an error when trying to read)?
> > > > > - Returning an empty value (nothing to see here, move along)?
> > > > No. This is confusing, to return empty value, because it says that
> > > > there is an
> > > alias but it is some weird empty string.
> > > > If there is alias, it shows exactly what it is.
> > > > If no alias it returns an error code = unsupported -> inline with
> > > > other widely
> > > used subsystem.
> > > >
> > > > > - Or not creating the attribute at all? That would match what userspace
> > > > > sees on older kernels, so it needs to be able to deal with
> > > > > that
> > > > New sysfs files can appear. Tool cannot say that I was not
> > > > expecting this file
> > > here.
> > > > User space is supposed to work with the file they are off interest.
> > > > Mdev interface has option to specify vendor specific files, though
> > > > in usual
> > > manner it's not recommended.
> > > > So there is no old user space, new kernel issue here.
> > >
> > > I'm not talking about old userspace/new kernel, but new userspace/old
> kernel.
> > > Code that wants to consume this attribute needs to be able to cope
> > > with its absence anyway.
> > >
> > Old kernel doesn't have alias file.
> > If some tool tries to read this file it will fail to open non existing file; open()
> system call is already taking care of it.
>
> Yes, that was exactly my argument?
I misunderstood probably.
I re-read all 3 options you posted.
I do not see any issue in reporting error code similar to other widely used netdev subsystem, hence propose what is posted in the patch.
^ permalink raw reply
* RE: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-27 11:57 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827134114.01ddd049.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, August 27, 2019 5:11 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
>
> On Tue, 27 Aug 2019 11:33:54 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 4:54 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > >
> > > On Tue, 27 Aug 2019 11:12:23 +0000
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > > Sent: Tuesday, August 27, 2019 3:54 PM
> > > > > To: Parav Pandit <parav@mellanox.com>
> > > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > > > >
>
> > > > > What about:
> > > > >
> > > > > * @get_alias_length: optional callback to specify length of the
> > > > > alias to
> > > create
> > > > > * Returns unsigned integer: length of the alias to be created,
> > > > > * 0 to not create an alias
> > > > >
> > > > Ack.
> > > >
> > > > > I also think it might be beneficial to add a device parameter
> > > > > here now (rather than later); that seems to be something that makes
> sense.
> > > > >
> > > > Without showing the use, it shouldn't be added.
> > >
> > > It just feels like an omission: Why should the vendor driver only be
> > > able to return one value here, without knowing which device it is for?
> > > If a driver supports different devices, it may have different
> > > requirements for them.
> > >
> > Sure. Lets first have this requirement to add it.
> > I am against adding this length field itself without an actual vendor use case,
> which is adding some complexity in code today.
> > But it was ok to have length field instead of bool.
> >
> > Lets not further add "no-requirement futuristic knobs" which hasn't shown its
> need yet.
> > When a vendor driver needs it, there is nothing prevents such addition.
>
> Frankly, I do not see how it adds complexity; the other callbacks have device
> arguments already,
Other ioctls such as create, remove, mmap, likely need to access the parent.
Hence it make sense to have parent pointer in there.
I am not against complexity, I am just saying, at present there is no use-case. Let have use case and we add it.
> and the vendor driver is free to ignore it if it does not have
> a use for it. I'd rather add the argument before a possible future user tries
> weird hacks to allow multiple values, but I'll leave the decision to the
> maintainers.
Why would a possible future user tries a weird hack?
If user needs to access parent device, that driver maintainer should ask for it.
^ permalink raw reply
* Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
From: Cornelia Huck @ 2019-08-27 11:55 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866FD2DB357C5EB4A7A75ADD1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:52:21 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 5:05 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> >
> > On Tue, 27 Aug 2019 11:07:37 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 4:17 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> > > >
> > > > On Mon, 26 Aug 2019 15:41:18 -0500
> > > > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > > +static ssize_t alias_show(struct device *device,
> > > > > + struct device_attribute *attr, char *buf) {
> > > > > + struct mdev_device *dev = mdev_from_dev(device);
> > > > > +
> > > > > + if (!dev->alias)
> > > > > + return -EOPNOTSUPP;
> > > >
> > > > I'm wondering how to make this consumable by userspace in the easiest
> > way.
> > > > - As you do now (userspace gets an error when trying to read)?
> > > > - Returning an empty value (nothing to see here, move along)?
> > > No. This is confusing, to return empty value, because it says that there is an
> > alias but it is some weird empty string.
> > > If there is alias, it shows exactly what it is.
> > > If no alias it returns an error code = unsupported -> inline with other widely
> > used subsystem.
> > >
> > > > - Or not creating the attribute at all? That would match what userspace
> > > > sees on older kernels, so it needs to be able to deal with that
> > > New sysfs files can appear. Tool cannot say that I was not expecting this file
> > here.
> > > User space is supposed to work with the file they are off interest.
> > > Mdev interface has option to specify vendor specific files, though in usual
> > manner it's not recommended.
> > > So there is no old user space, new kernel issue here.
> >
> > I'm not talking about old userspace/new kernel, but new userspace/old kernel.
> > Code that wants to consume this attribute needs to be able to cope with its
> > absence anyway.
> >
> Old kernel doesn't have alias file.
> If some tool tries to read this file it will fail to open non existing file; open() system call is already taking care of it.
Yes, that was exactly my argument?
^ permalink raw reply
* RE: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
From: Parav Pandit @ 2019-08-27 11:52 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827133432.156f7db3.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, August 27, 2019 5:05 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
>
> On Tue, 27 Aug 2019 11:07:37 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 4:17 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> > >
> > > On Mon, 26 Aug 2019 15:41:18 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
>
> > > > +static ssize_t alias_show(struct device *device,
> > > > + struct device_attribute *attr, char *buf) {
> > > > + struct mdev_device *dev = mdev_from_dev(device);
> > > > +
> > > > + if (!dev->alias)
> > > > + return -EOPNOTSUPP;
> > >
> > > I'm wondering how to make this consumable by userspace in the easiest
> way.
> > > - As you do now (userspace gets an error when trying to read)?
> > > - Returning an empty value (nothing to see here, move along)?
> > No. This is confusing, to return empty value, because it says that there is an
> alias but it is some weird empty string.
> > If there is alias, it shows exactly what it is.
> > If no alias it returns an error code = unsupported -> inline with other widely
> used subsystem.
> >
> > > - Or not creating the attribute at all? That would match what userspace
> > > sees on older kernels, so it needs to be able to deal with that
> > New sysfs files can appear. Tool cannot say that I was not expecting this file
> here.
> > User space is supposed to work with the file they are off interest.
> > Mdev interface has option to specify vendor specific files, though in usual
> manner it's not recommended.
> > So there is no old user space, new kernel issue here.
>
> I'm not talking about old userspace/new kernel, but new userspace/old kernel.
> Code that wants to consume this attribute needs to be able to cope with its
> absence anyway.
>
Old kernel doesn't have alias file.
If some tool tries to read this file it will fail to open non existing file; open() system call is already taking care of it.
^ permalink raw reply
* [PATCH] wimax/i2400m: remove redundant assignment to variable result
From: Colin King @ 2019-08-27 11:47 UTC (permalink / raw)
To: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Variable result is being assigned a value that is never read and result
is being re-assigned a little later on. The assignment is redundant
and hence can be removed.
Addresses-Coverity: ("Ununsed value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wimax/i2400m/rx.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index d28b96d06919..c9fb619a9e01 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -1253,7 +1253,6 @@ int i2400m_rx(struct i2400m *i2400m, struct sk_buff *skb)
skb_len = skb->len;
d_fnstart(4, dev, "(i2400m %p skb %p [size %u])\n",
i2400m, skb, skb_len);
- result = -EIO;
msg_hdr = (void *) skb->data;
result = i2400m_rx_msg_hdr_check(i2400m, msg_hdr, skb_len);
if (result < 0)
--
2.20.1
^ permalink raw reply related
* Re: BUG_ON in skb_segment, after bpf_skb_change_proto was applied
From: Shmulik Ladkani @ 2019-08-27 11:42 UTC (permalink / raw)
To: Eric Dumazet
Cc: Daniel Borkmann, netdev, Alexander Duyck, Alexei Starovoitov,
Yonghong Song, Steffen Klassert, shmulik, eyal
In-Reply-To: <94cd6f4d-09d4-11c0-64f4-bdc544bb3dcb@gmail.com>
On Mon, 26 Aug 2019 19:47:40 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 8/26/19 4:07 PM, Shmulik Ladkani wrote:
> > - ipv4 forwarding to dummy1, where eBPF nat4-to-6 program is attached
> > at TC Egress (calls 'bpf_skb_change_proto()'), then redirect to ingress
> > on same device.
> > NOTE: 'bpf_skb_proto_4_to_6()' mangles 'shinfo->gso_size'
>
> Doing this on an skb with a frag_list is doomed, in current gso_segment() state.
>
> A rewrite would be needed (I believe I did so at some point, but Herbert Xu fought hard against it)
Thanks Eric,
- If a rewrite is still considered out of the question, how can one use
eBPF's bpf_skb_change_proto() safely without disabling GRO completely?
- e.g. is there a way to force the GROed skbs to fit into a layout that is
tolerated by skb_segment?
- alternatively can eBPF layer somehow enforce segmentation of the
original GROed skb before mangling the gso_size?
- Another thing that puzzles me is that we hit the BUG_ON rather rarely
and cannot yet reproduce synthetically. If skb_segment's handling of
skbs with a frag_list (that have gso_size mangled) is broken, I'd expect
to hit this more often... Any ideas?
- Suppose going for a rewrite, care to elaborate what's exactly missing
in skb_segment's logic?
I must admit I do not fully understand all the different code flows in
this function, it seems to support many different input skbs - any
assistance is highly appreciated.
Shmulik
^ permalink raw reply
* Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-27 11:41 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866CC932630ADD9BDA51371D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:33:54 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 4:54 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> >
> > On Tue, 27 Aug 2019 11:12:23 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Tuesday, August 27, 2019 3:54 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > > >
> > > > What about:
> > > >
> > > > * @get_alias_length: optional callback to specify length of the alias to
> > create
> > > > * Returns unsigned integer: length of the alias to be created,
> > > > * 0 to not create an alias
> > > >
> > > Ack.
> > >
> > > > I also think it might be beneficial to add a device parameter here
> > > > now (rather than later); that seems to be something that makes sense.
> > > >
> > > Without showing the use, it shouldn't be added.
> >
> > It just feels like an omission: Why should the vendor driver only be able to
> > return one value here, without knowing which device it is for?
> > If a driver supports different devices, it may have different requirements for
> > them.
> >
> Sure. Lets first have this requirement to add it.
> I am against adding this length field itself without an actual vendor use case, which is adding some complexity in code today.
> But it was ok to have length field instead of bool.
>
> Lets not further add "no-requirement futuristic knobs" which hasn't shown its need yet.
> When a vendor driver needs it, there is nothing prevents such addition.
Frankly, I do not see how it adds complexity; the other callbacks have
device arguments already, and the vendor driver is free to ignore it if
it does not have a use for it. I'd rather add the argument before a
possible future user tries weird hacks to allow multiple values, but
I'll leave the decision to the maintainers.
^ permalink raw reply
* Re: [RFC PATCH net-next] net: phy: force phy suspend when calling phy_stop
From: shenjian (K) @ 2019-08-27 11:36 UTC (permalink / raw)
To: Sergei Shtylyov, andrew, f.fainelli, hkallweit1, davem
Cc: netdev, forest.zhouchang, linuxarm
In-Reply-To: <cc52cde5-b114-3bf8-4c4b-fe81c04080ee@cogentembedded.com>
在 2019/8/27 18:11, Sergei Shtylyov 写道:
> On 27.08.2019 5:47, Jian Shen wrote:
>
>> Some ethernet drivers may call phy_start() and phy_stop() from
>> ndo_open and ndo_close() respectively.
>
> ndo_open() for consistency.
>
>> When network cable is unconnected, and operate like below:
>> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
>> autoneg, and phy is no link.
>> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
>> phy state machine.
>> step 3: plugin the network cable, and autoneg complete, then
>> LED for link status will be on.
>> step 4: ethtool ethX --> see the result of "Link detected" is no.
>>
>> This patch forces phy suspend even phydev->link is off.
>>
>> Signed-off-by: Jian Shen <shenjian15@huawei.com>
> [...]
>
> MBR, Sergei
>
>
Thanks, will fix it.
^ permalink raw reply
* Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
From: Cornelia Huck @ 2019-08-27 11:34 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866BDA002F2C6566492244ED1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:07:37 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 4:17 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
> >
> > On Mon, 26 Aug 2019 15:41:18 -0500
> > Parav Pandit <parav@mellanox.com> wrote:
> > > +static ssize_t alias_show(struct device *device,
> > > + struct device_attribute *attr, char *buf) {
> > > + struct mdev_device *dev = mdev_from_dev(device);
> > > +
> > > + if (!dev->alias)
> > > + return -EOPNOTSUPP;
> >
> > I'm wondering how to make this consumable by userspace in the easiest way.
> > - As you do now (userspace gets an error when trying to read)?
> > - Returning an empty value (nothing to see here, move along)?
> No. This is confusing, to return empty value, because it says that there is an alias but it is some weird empty string.
> If there is alias, it shows exactly what it is.
> If no alias it returns an error code = unsupported -> inline with other widely used subsystem.
>
> > - Or not creating the attribute at all? That would match what userspace
> > sees on older kernels, so it needs to be able to deal with that
> New sysfs files can appear. Tool cannot say that I was not expecting this file here.
> User space is supposed to work with the file they are off interest.
> Mdev interface has option to specify vendor specific files, though in usual manner it's not recommended.
> So there is no old user space, new kernel issue here.
I'm not talking about old userspace/new kernel, but new userspace/old
kernel. Code that wants to consume this attribute needs to be able to
cope with its absence anyway.
>
> > anyway.
> >
> > > +
> > > + return sprintf(buf, "%s\n", dev->alias); } static
> > > +DEVICE_ATTR_RO(alias);
> > > +
> > > static const struct attribute *mdev_device_attrs[] = {
> > > + &dev_attr_alias.attr,
> > > &dev_attr_remove.attr,
> > > NULL,
> > > };
>
^ permalink raw reply
* RE: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-27 11:33 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190827132404.483a74ad.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Tuesday, August 27, 2019 4:54 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
>
> On Tue, 27 Aug 2019 11:12:23 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Tuesday, August 27, 2019 3:54 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> > >
> > > On Mon, 26 Aug 2019 15:41:16 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > Whenever a parent requests to generate mdev alias, generate a mdev
> > > > alias.
> > > > It is an optional attribute that parent can request to generate
> > > > for each of its child mdev.
> > > > mdev alias is generated using sha1 from the mdev name.
> > >
> > > Maybe add some motivation here as well?
> > >
> > > "Some vendor drivers want an identifier for an mdev device that is
> > > shorter than the uuid, due to length restrictions in the consumers of that
> identifier.
> > >
> > > Add a callback that allows a vendor driver to request an alias of a
> > > specified length to be generated (via sha1) for an mdev device. If
> > > generated, that alias is checked for collisions."
> > >
> > I did described the motivation in the cover letter with example and this
> design discussion thread.
>
> Yes, but adding it to the patch description makes it available in the git history.
>
Ok.
> > I will include above summary in v1.
> >
> > > What about:
> > >
> > > * @get_alias_length: optional callback to specify length of the alias to
> create
> > > * Returns unsigned integer: length of the alias to be created,
> > > * 0 to not create an alias
> > >
> > Ack.
> >
> > > I also think it might be beneficial to add a device parameter here
> > > now (rather than later); that seems to be something that makes sense.
> > >
> > Without showing the use, it shouldn't be added.
>
> It just feels like an omission: Why should the vendor driver only be able to
> return one value here, without knowing which device it is for?
> If a driver supports different devices, it may have different requirements for
> them.
>
Sure. Lets first have this requirement to add it.
I am against adding this length field itself without an actual vendor use case, which is adding some complexity in code today.
But it was ok to have length field instead of bool.
Lets not further add "no-requirement futuristic knobs" which hasn't shown its need yet.
When a vendor driver needs it, there is nothing prevents such addition.
> >
> > > > * Parent device that support mediated device should be
> > > > registered with
> > > mdev
> > > > * module with mdev_parent_ops structure.
> > > > **/
> > > > @@ -92,6 +95,7 @@ struct mdev_parent_ops {
> > > > long (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
> > > > unsigned long arg);
> > > > int (*mmap)(struct mdev_device *mdev, struct vm_area_struct
> > > *vma);
> > > > + unsigned int (*get_alias_length)(void);
> > > > };
> > > >
> > > > /* interface for exporting mdev supported type attributes */
> >
^ permalink raw reply
* [PATCH] arcnet: capmode: remove redundant assignment to pointer pkt
From: Colin King @ 2019-08-27 11:29 UTC (permalink / raw)
To: Michael Grzeschik, David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer pkt is being initialized with a value that is never read
and pkg is being re-assigned a little later on. The assignment is
redundant and hence can be removed.
Addresses-Coverity: ("Ununsed value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/arcnet/capmode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/arcnet/capmode.c b/drivers/net/arcnet/capmode.c
index b780be6f41ff..c09b567845e1 100644
--- a/drivers/net/arcnet/capmode.c
+++ b/drivers/net/arcnet/capmode.c
@@ -44,7 +44,7 @@ static void rx(struct net_device *dev, int bufnum,
{
struct arcnet_local *lp = netdev_priv(dev);
struct sk_buff *skb;
- struct archdr *pkt = pkthdr;
+ struct archdr *pkt;
char *pktbuf, *pkthdrbuf;
int ofs;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Cornelia Huck @ 2019-08-27 11:29 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB486621458EC71973378CD5A0D1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:08:59 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 3:59 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
> >
> > On Mon, 26 Aug 2019 15:41:17 -0500
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > Mdev alias should be unique among all the mdevs, so that when such
> > > alias is used by the mdev users to derive other objects, there is no
> > > collision in a given system.
> > >
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > ---
> > > drivers/vfio/mdev/mdev_core.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/vfio/mdev/mdev_core.c
> > > b/drivers/vfio/mdev/mdev_core.c index e825ff38b037..6eb37f0c6369
> > > 100644
> > > --- a/drivers/vfio/mdev/mdev_core.c
> > > +++ b/drivers/vfio/mdev/mdev_core.c
> > > @@ -375,6 +375,11 @@ int mdev_device_create(struct kobject *kobj, struct
> > device *dev,
> > > ret = -EEXIST;
> > > goto mdev_fail;
> > > }
> > > + if (tmp->alias && strcmp(tmp->alias, alias) == 0) {
> >
> > Any way we can relay to the caller that the uuid was fine, but that we had a
> > hash collision? Duplicate uuids are much more obvious than a collision here.
> >
> How do you want to relay this rare event?
> Netlink interface has way to return the error message back, but sysfs is limited due to its error code based interface.
I don't know, that's why I asked :)
The problem is that "uuid already used" and "hash collision" are
indistinguishable. While "use a different uuid" will probably work in
both cases, "increase alias length" might be a good alternative in some
cases.
But if there is no good way to relay the problem, we can live with it.
>
> > > + mutex_unlock(&mdev_list_lock);
> > > + ret = -EEXIST;
> > > + goto mdev_fail;
> > > + }
> > > }
> > >
> > > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
>
^ permalink raw reply
* Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-27 11:24 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866B68C9E60E42359BE1F4DD1A00@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 27 Aug 2019 11:12:23 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Tuesday, August 27, 2019 3:54 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
> >
> > On Mon, 26 Aug 2019 15:41:16 -0500
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > Whenever a parent requests to generate mdev alias, generate a mdev
> > > alias.
> > > It is an optional attribute that parent can request to generate for
> > > each of its child mdev.
> > > mdev alias is generated using sha1 from the mdev name.
> >
> > Maybe add some motivation here as well?
> >
> > "Some vendor drivers want an identifier for an mdev device that is shorter than
> > the uuid, due to length restrictions in the consumers of that identifier.
> >
> > Add a callback that allows a vendor driver to request an alias of a specified
> > length to be generated (via sha1) for an mdev device. If generated, that alias is
> > checked for collisions."
> >
> I did described the motivation in the cover letter with example and this design discussion thread.
Yes, but adding it to the patch description makes it available in the
git history.
> I will include above summary in v1.
>
> > What about:
> >
> > * @get_alias_length: optional callback to specify length of the alias to create
> > * Returns unsigned integer: length of the alias to be created,
> > * 0 to not create an alias
> >
> Ack.
>
> > I also think it might be beneficial to add a device parameter here now (rather
> > than later); that seems to be something that makes sense.
> >
> Without showing the use, it shouldn't be added.
It just feels like an omission: Why should the vendor driver only be
able to return one value here, without knowing which device it is for?
If a driver supports different devices, it may have different
requirements for them.
>
> > > * Parent device that support mediated device should be registered with
> > mdev
> > > * module with mdev_parent_ops structure.
> > > **/
> > > @@ -92,6 +95,7 @@ struct mdev_parent_ops {
> > > long (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
> > > unsigned long arg);
> > > int (*mmap)(struct mdev_device *mdev, struct vm_area_struct
> > *vma);
> > > + unsigned int (*get_alias_length)(void);
> > > };
> > >
> > > /* interface for exporting mdev supported type attributes */
>
^ 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