Re: [PATCH 2/2] Documentation: best practices for using Link trailers
On 18.06.24 18:42, Konstantin Ryabitsev wrote: > Based on multiple conversations, most recently on the ksummit mailing > list [1], add some best practices for using the Link trailer, such as: > > - how to use markdown-like bracketed numbers in the commit message to > indicate the corresponding link > - when to use lore.kernel.org vs patch.msgid.link domains [...] > + When using the ``Link:`` trailer to indicate the provenance of the > + patch, you should use the dedicated ``patch.msgid.link`` domain. This > + makes it possible for automated tooling to establish which link leads > + to the original patch submission. For example:: > + > + Link: https://patch.msgid.link/patch-source-msgid@here I wonder how long it will take until someone starts using patch.msgid.link/ for things that are not the submission of the change, for example by misunderstanding what "provenance of the patch" is meant to mean here. How about something this: """ In case you want to record the public review submission of a patch while committing it, use a ``Link:`` trailer with the dedicated ``patch.msgid.link`` domain:: Link: https://patch.msgid.link/patch-source-msgid@here This makes it possible to reliably look the submission up, hence don't use that domain for any other patches you might want to link to. """ But I suspect some people will never see this and start assuming that this domain should be meant for all patches -- and not all of these cases will be found during review (or by checkpatch, in case we add a check and people actually run it). Writing that made me think a dedicated tag like "Lore-Submission" or "Public-Review-Link" could avoid this while keeping some of the aspects that Linus likes about "Link" -- but I doubt that will convince him. Ciao, Thorsten
Re: [PATCH RESEND net-next v14 1/5] linux/dim: move useful macros to .h file
On Tue, Jun 18, 2024 at 10:56:40AM +0800, Heng Qi wrote: > Useful macros will be used effectively elsewhere. > These will be utilized in subsequent patches. > > Signed-off-by: Heng Qi Reviewed-by: Simon Horman
Re: [PATCH RESEND net-next v14 2/5] dim: make DIMLIB dependent on NET
On Tue, Jun 18, 2024 at 10:56:41AM +0800, Heng Qi wrote: > DIMLIB's capabilities are supplied by the dim, net_dim, and > rdma_dim objects, and dim's interfaces solely act as a base for > net_dim and rdma_dim and are not explicitly used anywhere else. > rdma_dim is utilized by the infiniband driver, while net_dim > is for network devices, excluding the soc/fsl driver. > > In this patch, net_dim relies on some NET's interfaces, thus > DIMLIB needs to explicitly depend on the NET Kconfig. > > The soc/fsl driver uses the functions provided by net_dim, so > it also needs to depend on NET. > > Signed-off-by: Heng Qi Reviewed-by: Simon Horman
Re: [PATCH RESEND net-next v14 3/5] ethtool: provide customized dim profile management
On Tue, Jun 18, 2024 at 10:56:42AM +0800, Heng Qi wrote:
> The NetDIM library, currently leveraged by an array of NICs, delivers
> excellent acceleration benefits. Nevertheless, NICs vary significantly
> in their dim profile list prerequisites.
>
> Specifically, virtio-net backends may present diverse sw or hw device
> implementation, making a one-size-fits-all parameter list impractical.
> On Alibaba Cloud, the virtio DPU's performance under the default DIM
> profile falls short of expectations, partly due to a mismatch in
> parameter configuration.
>
> I also noticed that ice/idpf/ena and other NICs have customized
> profilelist or placed some restrictions on dim capabilities.
>
> Motivated by this, I tried adding new params for "ethtool -C" that provides
> a per-device control to modify and access a device's interrupt parameters.
>
> Usage
>
> The target NIC is named ethx.
>
> Assume that ethx only declares support for rx profile setting
> (with DIM_PROFILE_RX flag set in profile_flags) and supports modification
> of usec and pkt fields.
>
> 1. Query the currently customized list of the device
>
> $ ethtool -c ethx
> ...
> rx-profile:
> {.usec = 1, .pkts = 256, .comps = n/a,},
> {.usec = 8, .pkts = 256, .comps = n/a,},
> {.usec = 64, .pkts = 256, .comps = n/a,},
> {.usec = 128, .pkts = 256, .comps = n/a,},
> {.usec = 256, .pkts = 256, .comps = n/a,}
> tx-profile: n/a
>
> 2. Tune
> $ ethtool -C ethx rx-profile 1,1,n_2,n,n_3,3,n_4,4,n_n,5,n
> "n" means do not modify this field.
> $ ethtool -c ethx
> ...
> rx-profile:
> {.usec = 1, .pkts = 1, .comps = n/a,},
> {.usec = 2, .pkts = 256, .comps = n/a,},
> {.usec = 3, .pkts = 3, .comps = n/a,},
> {.usec = 4, .pkts = 4, .comps = n/a,},
> {.usec = 256, .pkts = 5, .comps = n/a,}
> tx-profile: n/a
>
> 3. Hint
> If the device does not support some type of customized dim profiles,
> the corresponding "n/a" will display.
>
> If the "n/a" field is being modified, -EOPNOTSUPP will be reported.
>
> Signed-off-by: Heng Qi
Reviewed-by: Simon Horman
Re: [PATCH RESEND net-next v14 4/5] dim: add new interfaces for initialization and getting results
On Tue, Jun 18, 2024 at 10:56:43AM +0800, Heng Qi wrote: > DIM-related mode and work have been collected in one same place, > so new interfaces are added to provide convenience. > > Signed-off-by: Heng Qi Reviewed-by: Simon Horman
Re: [PATCH RESEND net-next v14 5/5] virtio-net: support dim profile fine-tuning
On Tue, Jun 18, 2024 at 10:56:44AM +0800, Heng Qi wrote: > Virtio-net has different types of back-end device implementations. > In order to effectively optimize the dim library's gains for different > device implementations, let's use the new interface params to > initialize and query dim results from a customized profile list. > > Signed-off-by: Heng Qi Reviewed-by: Simon Horman
Re: [PATCH v6 2/2] proc: restrict /proc/pid/mem
On Wed, Jun 19, 2024 at 1:41 PM Kees Cook wrote:
>
> On Tue, Jun 18, 2024 at 03:39:44PM -0700, Jeff Xu wrote:
> > Hi
> >
> > Thanks for the patch !
> >
> > On Thu, Jun 13, 2024 at 6:40 AM Adrian Ratiu
> > wrote:
> > >
> > > Prior to v2.6.39 write access to /proc//mem was restricted,
> > > after which it got allowed in commit 198214a7ee50 ("proc: enable
> > > writing to /proc/pid/mem"). Famous last words from that patch:
> > > "no longer a security hazard". :)
> > >
> > > Afterwards exploits started causing drama like [1]. The exploits
> > > using /proc/*/mem can be rather sophisticated like [2] which
> > > installed an arbitrary payload from noexec storage into a running
> > > process then exec'd it, which itself could include an ELF loader
> > > to run arbitrary code off noexec storage.
> > >
> > > One of the well-known problems with /proc/*/mem writes is they
> > > ignore page permissions via FOLL_FORCE, as opposed to writes via
> > > process_vm_writev which respect page permissions. These writes can
> > > also be used to bypass mode bits.
> > >
> > > To harden against these types of attacks, distrbutions might want
> > > to restrict /proc/pid/mem accesses, either entirely or partially,
> > > for eg. to restrict FOLL_FORCE usage.
> > >
> > > Known valid use-cases which still need these accesses are:
> > >
> > > * Debuggers which also have ptrace permissions, so they can access
> > > memory anyway via PTRACE_POKEDATA & co. Some debuggers like GDB
> > > are designed to write /proc/pid/mem for basic functionality.
> > >
> > > * Container supervisors using the seccomp notifier to intercept
> > > syscalls and rewrite memory of calling processes by passing
> > > around /proc/pid/mem file descriptors.
> > >
> > > There might be more, that's why these params default to disabled.
> > >
> > > Regarding other mechanisms which can block these accesses:
> > >
> > > * seccomp filters can be used to block mmap/mprotect calls with W|X
> > > perms, but they often can't block open calls as daemons want to
> > > read/write their runtime state and seccomp filters cannot check
> > > file paths, so plain write calls can't be easily blocked.
> > >
> > > * Since the mem file is part of the dynamic /proc// space, we
> > > can't run chmod once at boot to restrict it (and trying to react
> > > to every process and run chmod doesn't scale, and the kernel no
> > > longer allows chmod on any of these paths).
> > >
> > > * SELinux could be used with a rule to cover all /proc/*/mem files,
> > > but even then having multiple ways to deny an attack is useful in
> > > case one layer fails.
> > >
> > > Thus we introduce four kernel parameters to restrict /proc/*/mem
> > > access: open-read, open-write, write and foll_force. All these can
> > > be independently set to the following values:
> > >
> > > all => restrict all access unconditionally.
> > > ptracer => restrict all access except for ptracer processes.
> > >
> > > If left unset, the existing behaviour is preserved, i.e. access
> > > is governed by basic file permissions.
> > >
> > > Examples which can be passed by bootloaders:
> > >
> > > proc_mem.restrict_foll_force=all
> > > proc_mem.restrict_open_write=ptracer
> > > proc_mem.restrict_open_read=ptracer
> > > proc_mem.restrict_write=all
> > >
> > > These knobs can also be enabled via Kconfig like for eg:
> > >
> > > CONFIG_PROC_MEM_RESTRICT_WRITE_PTRACE_DEFAULT=y
> > > CONFIG_PROC_MEM_RESTRICT_FOLL_FORCE_PTRACE_DEFAULT=y
> > >
> > > Each distribution needs to decide what restrictions to apply,
> > > depending on its use-cases. Embedded systems might want to do
> > > more, while general-purpouse distros might want a more relaxed
> > > policy, because for e.g. foll_force=all and write=all both break
> > > break GDB, so it might be a bit excessive.
> > >
> > > Based on an initial patch by Mike Frysinger .
> > >
> > It is noteworthy that ChromeOS has benefited from blocking
> > /proc/pid/mem write since 2017 [1], owing to the patch implemented by
> > Mike Frysinger.
> >
> > It is great that upstream can consider this patch, ChromeOS will use
> > the solution once it is accepted.
> >
> > > Link: https://lwn.net/Articles/476947/ [1]
> > > Link: https://issues.chromium.org/issues/40089045 [2]
> > > Cc: Guenter Roeck
> > > Cc: Doug Anderson
> > > Cc: Kees Cook
> > > Cc: Jann Horn
> > > Cc: Andrew Morton
> > > Cc: Randy Dunlap
> > > Cc: Christian Brauner
> > > Cc: Jeff Xu
> > > Co-developed-by: Mike Frysinger
> > > Signed-off-by: Mike Frysinger
> > > Signed-off-by: Adrian Ratiu
> >
> > Reviewed-by: Jeff Xu
> > Tested-by: Jeff Xu
> > [1]
> > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/764773
>
> Thanks for the testing! What settings did you use? I think Chrome OS was
> effectively doing this?
>
> PROC_MEM_RESTRICT_OPEN_READ_OFF=y
> CONFIG_PROC_MEM_RESTRICT_OPEN_WRITE_ALL=y
> CONFIG_PROC_MEM_RESTRICT_WRITE_ALL=y
> CONFIG_PROC_MEM_RESTRICT_FOLL_FORCE_ALL=y
>
> Though I don't see the FOLL_FORC
Re: [PATCH RESEND net-next v14 3/5] ethtool: provide customized dim profile management
On Tue, 18 Jun 2024 10:56:42 +0800 Heng Qi wrote:
> + if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_RX) {
> + ret = ethnl_update_profile(dev, &dev->irq_moder->rx_profile,
> +tb[ETHTOOL_A_COALESCE_RX_PROFILE],
> +info->extack);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_TX) {
> + ret = ethnl_update_profile(dev, &dev->irq_moder->tx_profile,
> +tb[ETHTOOL_A_COALESCE_TX_PROFILE],
> +info->extack);
> + if (ret < 0)
> + return ret;
> + }
One last thing - you're missing updating the &mod bit.
When any of the settings were change mod should be set
to true so that we send a notification to user space,
that the settings have been modified.
Re: [PATCH RESEND net-next v14 0/5] ethtool: provide the dim profile fine-tuning channel
On Tue, 18 Jun 2024 10:56:39 +0800 Heng Qi wrote: > The NetDIM library provides excellent acceleration for many modern > network cards. However, the default profiles of DIM limits its maximum > capabilities for different NICs, so providing a way which the NIC can > be custom configured is necessary. Could you give an example in the cover letter of a type of workload and how much a new set of DIM values helped?
Re: [PATCH RESEND net-next v14 3/5] ethtool: provide customized dim profile management
On Tue, 18 Jun 2024 10:56:42 +0800 Heng Qi wrote: > --- a/Documentation/networking/ethtool-netlink.rst > +++ b/Documentation/networking/ethtool-netlink.rst > @@ -1033,6 +1033,8 @@ Kernel response contents: >``ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES`` u32 max aggr size, Tx >``ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES``u32 max aggr packets, Tx >``ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS``u32 time (us), aggr, Tx > + ``ETHTOOL_A_COALESCE_RX_PROFILE``nested profile of DIM, Rx > + ``ETHTOOL_A_COALESCE_TX_PROFILE``nested profile of DIM, Tx >=== == > === > > Attributes are only included in reply if their value is not zero or the Maybe add a short line in the section for COALESCE_GET linking to dim? Something like: ``ETHTOOL_A_COALESCE_RX_PROFILE`` and ``ETHTOOL_A_COALESCE_TX_PROFILE`` refer to DIM parameters, see ... <- add ReST link to net_dim.rst here.
Re: [PATCH RESEND net-next v14 3/5] ethtool: provide customized dim profile management
On Thu, 20 Jun 2024 20:39:18 -0700, Jakub Kicinski wrote:
> On Tue, 18 Jun 2024 10:56:42 +0800 Heng Qi wrote:
> > + if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_RX) {
> > + ret = ethnl_update_profile(dev, &dev->irq_moder->rx_profile,
> > + tb[ETHTOOL_A_COALESCE_RX_PROFILE],
> > + info->extack);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + if (dev->irq_moder && dev->irq_moder->profile_flags & DIM_PROFILE_TX) {
> > + ret = ethnl_update_profile(dev, &dev->irq_moder->tx_profile,
> > + tb[ETHTOOL_A_COALESCE_TX_PROFILE],
> > + info->extack);
> > + if (ret < 0)
> > + return ret;
> > + }
>
> One last thing - you're missing updating the &mod bit.
> When any of the settings were change mod should be set
> to true so that we send a notification to user space,
> that the settings have been modified.
Oh, I didn't modify the mod bit in the past because the profile list
modification does not require the dual_change behavior, ignoring the
passing of ret = 0/1. Will modify.
Thanks.
Re: [PATCH RESEND net-next v14 3/5] ethtool: provide customized dim profile management
On Thu, 20 Jun 2024 20:44:45 -0700, Jakub Kicinski wrote: > On Tue, 18 Jun 2024 10:56:42 +0800 Heng Qi wrote: > > --- a/Documentation/networking/ethtool-netlink.rst > > +++ b/Documentation/networking/ethtool-netlink.rst > > @@ -1033,6 +1033,8 @@ Kernel response contents: > >``ETHTOOL_A_COALESCE_TX_AGGR_MAX_BYTES`` u32 max aggr size, Tx > >``ETHTOOL_A_COALESCE_TX_AGGR_MAX_FRAMES``u32 max aggr packets, Tx > >``ETHTOOL_A_COALESCE_TX_AGGR_TIME_USECS``u32 time (us), aggr, Tx > > + ``ETHTOOL_A_COALESCE_RX_PROFILE``nested profile of DIM, Rx > > + ``ETHTOOL_A_COALESCE_TX_PROFILE``nested profile of DIM, Tx > >=== == > > === > > > > Attributes are only included in reply if their value is not zero or the > > Maybe add a short line in the section for COALESCE_GET linking to dim? > Something like: > > ``ETHTOOL_A_COALESCE_RX_PROFILE`` and ``ETHTOOL_A_COALESCE_TX_PROFILE`` > refer to DIM parameters, see ... <- add ReST link to net_dim.rst here. Will add. Thanks.
Re: [PATCH RESEND net-next v14 0/5] ethtool: provide the dim profile fine-tuning channel
On Thu, 20 Jun 2024 20:40:25 -0700, Jakub Kicinski wrote: > On Tue, 18 Jun 2024 10:56:39 +0800 Heng Qi wrote: > > The NetDIM library provides excellent acceleration for many modern > > network cards. However, the default profiles of DIM limits its maximum > > capabilities for different NICs, so providing a way which the NIC can > > be custom configured is necessary. > > Could you give an example in the cover letter of a type of workload and > how much a new set of DIM values helped? Sure.
