* 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
* Re: [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs
From: Florian Westphal @ 2019-08-27 11:18 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: Florian Westphal, Pablo Neira Ayuso, netdev, shmulik
In-Reply-To: <20190827135754.7d460ef8@pixies>
Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> -static int nf_conntrack_acct_init_sysctl(struct net *net)
> -{
> - struct ctl_table *table;
> -
> - table = kmemdup(acct_sysctl_table, sizeof(acct_sysctl_table),
> - GFP_KERNEL);
> - if (!table)
> - goto out;
> -
> - table[0].data = &net->ct.sysctl_acct;
> -
>
> (where 'nf_conntrack_acct_init_sysctl()' was originally called by
> 'nf_conntrack_acct_pernet_init()').
>
> However POST d912dec12428, the per-net netfilter sysctl table simply
> inherits from global 'nf_ct_sysctl_table[]', which has
>
> + .data = &init_net.ct.sysctl_acct,
>
> effectivly making any 'net.netfilter.nf_conntrack_acct' sysctl change
> affect the 'init_net' and not relevant net namespace.
>
> Also, looks like "nf_conntrack_helper", "nf_conntrack_events",
> "nf_conntrack_timestamp" where also harmed in a similar way, see:
>
> d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
> cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
Thanks for reporting this bug, I will submit a patch soon.
^ permalink raw reply
* RE: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-27 11:16 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: <20190827122428.37442fe1.cohuck@redhat.com>
> -----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:
> >
> > static int __init mdev_init(void)
> > {
> > + alias_hash = crypto_alloc_shash("sha1", 0, 0);
> > + if (!alias_hash)
> > + return -ENOMEM;
> > +
> > return mdev_bus_register();
>
> Don't you need to call crypto_free_shash() if mdev_bus_register() fails?
>
Missed to answer this in previous reply.
Yes, took care of it in v1.
Mark Bloch also pointed it to me.
^ permalink raw reply
* RE: [PATCH 1/4] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-27 11:12 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: <20190827122428.37442fe1.cohuck@redhat.com>
> -----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.
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.
> > * 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
* RE: [PATCH 2/4] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-27 11:08 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: <20190827122928.752e763b.cohuck@redhat.com>
> -----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.
> > + mutex_unlock(&mdev_list_lock);
> > + ret = -EEXIST;
> > + goto mdev_fail;
> > + }
> > }
> >
> > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
^ permalink raw reply
* RE: [PATCH 3/4] mdev: Expose mdev alias in sysfs tree
From: Parav Pandit @ 2019-08-27 11:07 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: <20190827124706.7e726794.cohuck@redhat.com>
> -----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:
>
> > Expose mdev alias as string in a sysfs tree so that such attribute can
> > be used to generate netdevice name by systemd/udev or can be used to
> > match other kernel objects based on the alias of the mdev.
>
> What about
>
> "Expose the optional alias for an mdev device as a sysfs attribute.
> This way, userspace tools such as udev may make use of the alias, for example
> to create a netdevice name for the mdev."
>
Ok. I will change it.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> > drivers/vfio/mdev/mdev_sysfs.c | 13 +++++++++++++
>
> I think the documentation should be updated as well.
>
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/vfio/mdev/mdev_sysfs.c
> > b/drivers/vfio/mdev/mdev_sysfs.c index 43afe0e80b76..59f4e3cc5233
> > 100644
> > --- a/drivers/vfio/mdev/mdev_sysfs.c
> > +++ b/drivers/vfio/mdev/mdev_sysfs.c
> > @@ -246,7 +246,20 @@ static ssize_t remove_store(struct device *dev,
> > struct device_attribute *attr,
> >
> > static DEVICE_ATTR_WO(remove);
> >
> > +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.
> 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
* SFP+ EEPROM readouts fail on X722 (ethtool -m: Invalid argument)
From: Jakub Jankowski @ 2019-08-27 11:03 UTC (permalink / raw)
To: e1000-devel; +Cc: shasta, mhemsley, netdev
Hi,
We can't get SFP+ EEPROM readouts for X722 to work at all:
# ethtool -m eth10
Cannot get module EEPROM information: Invalid argument
# dmesg | tail -n 1
[ 445.971974] i40e 0000:3d:00.3 eth10: Module EEPROM memory read not supported. Please update the NVM image.
# lspci | grep 3d:00.3
3d:00.3 Ethernet controller: Intel Corporation Ethernet Connection X722 for 10GbE SFP+ (rev 09)
We're running 4.19.65 kernel at the moment, testing using the newest out-of-tree Intel module
# modinfo -F version i40e
2.9.21
We also tried:
- 4.19.65 with in-tree i40e (2.3.2-k)
- stock Arch Linux (kernel 5.2.5, driver 2.8.20-k)
and the results are the same, as shown above.
# ethtool -i eth10
driver: i40e
version: 2.9.21
firmware-version: 3.31 0x80000d31 1.1767.0
expansion-rom-version:
bus-info: 0000:3d:00.3
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: yes
# dmidecode -s baseboard-manufacturer
Intel Corporation
# dmidecode -s baseboard-product-name
S2600WFT
# dmidecode -s baseboard-version
H48104-853
# lspci -vvv
(...)
3d:00.3 Ethernet controller: Intel Corporation Ethernet Connection X722 for 10GbE SFP+ (rev 09)
DeviceName: Intel PCH Integrated 10 Gigabit Ethernet Controller
Subsystem: Intel Corporation Ethernet Connection X722 for 10GbE SFP+
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0, Cache Line Size: 32 bytes
Interrupt: pin A routed to IRQ 112
NUMA node: 0
Region 0: Memory at ab000000 (64-bit, prefetchable) [size=16M]
Region 3: Memory at b0000000 (64-bit, prefetchable) [size=32K]
Expansion ROM at <ignored> [disabled]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=1 PME-
Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
Address: 0000000000000000 Data: 0000
Masking: 00000000 Pending: 00000000
Capabilities: [70] MSI-X: Enable+ Count=129 Masked-
Vector table: BAR=3 offset=00000000
PBA: BAR=3 offset=00001000
Capabilities: [a0] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+ SlotPowerLimit 0.000W
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop- FLReset-
MaxPayload 256 bytes, MaxReadReq 512 bytes
DevSta: CorrErr+ NonFatalErr- FatalErr- UnsupReq+ AuxPwr+ TransPend-
LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <64ns, L1 <1us
ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp+
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s (ok), Width x1 (ok)
TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
DevCap2: Completion Timeout: Range AB, TimeoutDis+, LTR-, OBFF Not Supported
AtomicOpsCap: 32bit- 64bit- 128bitCAS-
DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
AtomicOpsCtl: ReqEn-
LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
Capabilities: [e0] Vital Product Data
Product Name: Example VPD
Read-only fields:
[V0] Vendor specific:
[RV] Reserved: checksum good, 0 byte(s) reserved
End
Capabilities: [100 v2] Advanced Error Reporting
UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSViol-
UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+
CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+
AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn-
MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap-
HeaderLog: 00000000 00000000 00000000 00000000
Capabilities: [150 v1] Alternative Routing-ID Interpretation (ARI)
ARICap: MFVC- ACS-, Next Function: 0
ARICtl: MFVC- ACS-, Function Group: 0
Capabilities: [160 v1] Single Root I/O Virtualization (SR-IOV)
IOVCap: Migration-, Interrupt Message Number: 000
IOVCtl: Enable- Migration- Interrupt- MSE- ARIHierarchy-
IOVSta: Migration-
Initial VFs: 32, Total VFs: 32, Number of VFs: 0, Function Dependency Link: 03
VF offset: 109, stride: 1, Device ID: 37cd
Supported Page Size: 00000553, System Page Size: 00000001
Region 0: Memory at 00000000af000000 (64-bit, prefetchable)
Region 3: Memory at 00000000b0020000 (64-bit, prefetchable)
VF Migration: offset: 00000000, BIR: 0
Capabilities: [1a0 v1] Transaction Processing Hints
Device specific mode supported
No steering table available
Capabilities: [1b0 v1] Access Control Services
ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans-
ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans-
Kernel driver in use: i40e
Kernel modules: i40e
Same kernel+i40e, same SFP+ module - but on Intel X710, works like a treat:
# lspci | grep X7
81:00.0 Ethernet controller: Intel Corporation Ethernet Controller X710 for 10GbE SFP+ (rev 01)
81:00.1 Ethernet controller: Intel Corporation Ethernet Controller X710 for 10GbE SFP+ (rev 01)
# ethtool -m eth8
Identifier : 0x03 (SFP)
Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
Connector : 0x07 (LC)
Transceiver codes : 0x10 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00
Transceiver type : 10G Ethernet: 10G Base-SR
Transceiver type : Ethernet: 1000BASE-SX
Encoding : 0x06 (64B/66B)
BR, Nominal : 10300MBd
(...)
# ethtool -i eth8
driver: i40e
version: 2.9.21
firmware-version: 6.01 0x800035cf 1.1876.0
expansion-rom-version:
bus-info: 0000:81:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: yes
#
Is this a known problem?
Best regards,
Jakub
^ permalink raw reply
* Re: [PATCH net v3] ixgbe: fix double clean of tx descriptors with xdp
From: Ilya Maximets @ 2019-08-27 11:00 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, linux-kernel, bpf, David S. Miller, Björn Töpel,
Magnus Karlsson, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jeff Kirsher, intel-wired-lan, Eelco Chaudron,
William Tu, Alexander Duyck
In-Reply-To: <20190826154042.00004bfc@gmail.com>
On 26.08.2019 16:40, Maciej Fijalkowski wrote:
> On Thu, 22 Aug 2019 20:12:37 +0300
> Ilya Maximets <i.maximets@samsung.com> wrote:
>
>> Tx code doesn't clear the descriptors' status after cleaning.
>> So, if the budget is larger than number of used elems in a ring, some
>> descriptors will be accounted twice and xsk_umem_complete_tx will move
>> prod_tail far beyond the prod_head breaking the completion queue ring.
>>
>> Fix that by limiting the number of descriptors to clean by the number
>> of used descriptors in the tx ring.
>>
>> 'ixgbe_clean_xdp_tx_irq()' function refactored to look more like
>> 'ixgbe_xsk_clean_tx_ring()' since we're allowed to directly use
>> 'next_to_clean' and 'next_to_use' indexes.
>>
>> Fixes: 8221c5eba8c1 ("ixgbe: add AF_XDP zero-copy Tx support")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>
>> Version 3:
>> * Reverted some refactoring made for v2.
>> * Eliminated 'budget' for tx clean.
>> * prefetch returned.
>>
>> Version 2:
>> * 'ixgbe_clean_xdp_tx_irq()' refactored to look more like
>> 'ixgbe_xsk_clean_tx_ring()'.
>>
>> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 29 ++++++++------------
>> 1 file changed, 11 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> index 6b609553329f..a3b6d8c89127 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
>> @@ -633,19 +633,17 @@ static void ixgbe_clean_xdp_tx_buffer(struct ixgbe_ring *tx_ring,
>> bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
>> struct ixgbe_ring *tx_ring, int napi_budget)
>
> While you're at it, can you please as well remove the 'napi_budget' argument?
> It wasn't used at all even before your patch.
As you mentioned, this is not related to current patch and this patch doesn't
touch these particular lines of code. So, I think it's better to make a
separate patch for this if you think it's needed.
>
> I'm jumping late in, but I was really wondering and hesitated with taking
> part in discussion since the v1 of this patch - can you elaborate why simply
> clearing the DD bit wasn't sufficient?
Clearing the DD bit will end up driver and hardware writing to close memory
locations at the same time leading to cache trashing and poor performance.
Anyway additional write is unnecessary, because we know exactly which descriptors
we need to check.
Best regards, Ilya Maximets.
>
> Maciej
>
>> {
>> + u16 ntc = tx_ring->next_to_clean, ntu = tx_ring->next_to_use;
>> unsigned int total_packets = 0, total_bytes = 0;
>> - u32 i = tx_ring->next_to_clean, xsk_frames = 0;
>> - unsigned int budget = q_vector->tx.work_limit;
>> struct xdp_umem *umem = tx_ring->xsk_umem;
>> union ixgbe_adv_tx_desc *tx_desc;
>> struct ixgbe_tx_buffer *tx_bi;
>> - bool xmit_done;
>> + u32 xsk_frames = 0;
>>
>> - tx_bi = &tx_ring->tx_buffer_info[i];
>> - tx_desc = IXGBE_TX_DESC(tx_ring, i);
>> - i -= tx_ring->count;
>> + tx_bi = &tx_ring->tx_buffer_info[ntc];
>> + tx_desc = IXGBE_TX_DESC(tx_ring, ntc);
>>
>> - do {
>> + while (ntc != ntu) {
>> if (!(tx_desc->wb.status & cpu_to_le32(IXGBE_TXD_STAT_DD)))
>> break;
>>
>> @@ -661,22 +659,18 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
>>
>> tx_bi++;
>> tx_desc++;
>> - i++;
>> - if (unlikely(!i)) {
>> - i -= tx_ring->count;
>> + ntc++;
>> + if (unlikely(ntc == tx_ring->count)) {
>> + ntc = 0;
>> tx_bi = tx_ring->tx_buffer_info;
>> tx_desc = IXGBE_TX_DESC(tx_ring, 0);
>> }
>>
>> /* issue prefetch for next Tx descriptor */
>> prefetch(tx_desc);
>> + }
>>
>> - /* update budget accounting */
>> - budget--;
>> - } while (likely(budget));
>> -
>> - i += tx_ring->count;
>> - tx_ring->next_to_clean = i;
>> + tx_ring->next_to_clean = ntc;
>>
>> u64_stats_update_begin(&tx_ring->syncp);
>> tx_ring->stats.bytes += total_bytes;
>> @@ -688,8 +682,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
>> if (xsk_frames)
>> xsk_umem_complete_tx(umem, xsk_frames);
>>
>> - xmit_done = ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
>> - return budget > 0 && xmit_done;
>> + return ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
>> }
>>
>> int ixgbe_xsk_async_xmit(struct net_device *dev, u32 qid)
>
>
>
^ permalink raw reply
* [REGRESSION] netfilter: conntrack: Unable to change conntrack accounting of a net namespace via 'nf_conntrack_acct' sysfs
From: Shmulik Ladkani @ 2019-08-27 10:57 UTC (permalink / raw)
To: Florian Westphal, Pablo Neira Ayuso; +Cc: netdev, shmulik
Hi,
Prior d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
one was able to enable extended accounting within a (non-init)
net-namespace by setting: 'net.netfilter.nf_conntrack_acct=1'
However since d912dec12428, doing so results in changing init_net's
sysctl_acct field, instead of the relevant net's sysctl_acct.
Seen in original code, PRE d912dec12428, which creates a reference to
each net's _OWN_ ct.sysctl_acct within a separate acct_sysctl_table,
snip:
-static int nf_conntrack_acct_init_sysctl(struct net *net)
-{
- struct ctl_table *table;
-
- table = kmemdup(acct_sysctl_table, sizeof(acct_sysctl_table),
- GFP_KERNEL);
- if (!table)
- goto out;
-
- table[0].data = &net->ct.sysctl_acct;
-
(where 'nf_conntrack_acct_init_sysctl()' was originally called by
'nf_conntrack_acct_pernet_init()').
However POST d912dec12428, the per-net netfilter sysctl table simply
inherits from global 'nf_ct_sysctl_table[]', which has
+ .data = &init_net.ct.sysctl_acct,
effectivly making any 'net.netfilter.nf_conntrack_acct' sysctl change
affect the 'init_net' and not relevant net namespace.
Also, looks like "nf_conntrack_helper", "nf_conntrack_events",
"nf_conntrack_timestamp" where also harmed in a similar way, see:
d912dec12428 ("netfilter: conntrack: merge acct and helper sysctl table with main one")
cb2833ed0044 ("netfilter: conntrack: merge ecache and timestamp sysctl tables with main one")
Florian, would it be possible for you to revert these on -net ?
Many thanks,
Shmulik
^ 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