All of lore.kernel.org
 help / color / mirror / Atom feed
From: Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: mkubecek@suse.cz, netdev@vger.kernel.org
Subject: Re: [PATCH ethtool] netlink: settings: fix netlink support when PLCA is not present
Date: Tue, 25 Apr 2023 12:33:12 +0200	[thread overview]
Message-ID: <ZEesaPog588QIRfL@gvm01> (raw)
In-Reply-To: <20230425000742.130480-1-kuba@kernel.org>

On Mon, Apr 24, 2023 at 05:07:42PM -0700, Jakub Kicinski wrote:
> PLCA support threw the PLCA commands as required into the initial
> support check at the start of nl_gset(). That's not correct.
> The initial check (AFAIU) queries for the base support in the kernel
> i.e. support for the commands which correspond to ioctls.
> If those are not available (presumably very old kernel or kernel
> without ethtool-netlink) we're better off using the ioctl.
> 
> For new functionality, however, falling back to ioctl
> is counterproductive. New functionality (like PLCA) isn't
> supported via the ioctl, anyway, and we're losing all the other
> netlink-only functionality (I noticed that the link down statistics
> are gone).
> 
> After much deliberation I decided to add a second check for
> command support in gset_request(). Seems cleanest and if any
> of the non-required commands narrows the capabilities (e.g.
> does not support dump) we should just skip it too. Falling
> back to ioctl would again be a regression.
Thanks Jackub, that makes sense to me (FWIW).
I'm trying this patch on my system and I can see it does not create an
issue on systems where PLCA is -not- supported.

However, as soon as I try this on a system where PLCA is enabled, I get
a segmentation fault of ethtool. I'm currently investigating the reason.

Thanks,
Piergiorgio


  reply	other threads:[~2023-04-25 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25  0:07 [PATCH ethtool] netlink: settings: fix netlink support when PLCA is not present Jakub Kicinski
2023-04-25 10:33 ` Piergiorgio Beruto [this message]
2023-04-25 14:40 ` Piergiorgio Beruto
2023-04-25 15:34   ` Jakub Kicinski
2023-05-07 22:40 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZEesaPog588QIRfL@gvm01 \
    --to=piergiorgio.beruto@gmail.com \
    --cc=kuba@kernel.org \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.