netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Gal Pressman <gal@nvidia.com>
Cc: Edwin Peer <espeer@gmail.com>, David Ahern <dsahern@gmail.com>,
	netdev <netdev@vger.kernel.org>,
	Andrew Gospodarek <andrew.gospodarek@broadcom.com>,
	Michael Chan <michael.chan@broadcom.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Michal Kubecek <mkubecek@suse.cz>
Subject: Re: [PATCH net-next 1/4] netlink: truncate overlength attribute list in nla_nest_end()
Date: Wed, 7 Jun 2023 09:33:24 -0700	[thread overview]
Message-ID: <20230607093324.2b7712d9@kernel.org> (raw)
In-Reply-To: <f2a02c4f-a9c0-a586-1bde-ff2779933270@nvidia.com>

On Wed, 7 Jun 2023 16:31:48 +0300 Gal Pressman wrote:
> On 06/06/2023 19:17, Jakub Kicinski wrote:
> > On Tue, 6 Jun 2023 11:01:14 +0300 Gal Pressman wrote:  
> >> Jakub, sorry if this has been discussed already in the past, but can you
> >> please clarify what is an accepted (or more importantly, not accepted)
> >> solution for this issue? I'm not familiar with the history and don't
> >> want to repeat previous mistakes.  
> > 
> > The problem is basically that attributes can only be 64kB and 
> > the legacy SR-IOV API wraps all the link info in an attribute.  
> 
> Isn't that a second order issue? The skb itself is limited to 32kB AFAICT.

Hm, you're right. But allocation larger than 32kB are costly.
We can't make every link dump allocate 64kB, it will cause
regressions on systems under memory pressure (== real world).

You'd need to come up with some careful scheme of using larger
buffers.

> >> So far I've seen discussions about increasing the recv buffer size, and
> >> this patchset which changes the GETLINK ABI, both of which were nacked.  
> > 
> > Filtering out some of the info, like the stats, is okay, but that just
> > increases the limit. A limit still exists.  
> 
> Any objections to at least take the second patch here?
> It doesn't introduce any ABI changes, but will allow 'ip link show' to
> work properly (although 'ip -s link show' will remain broken).

Yup, retest / repost?

> >> Having 'ip link show' broken is very unfortunate :\, how should one
> >> approach this issue in 2023?  
> > 
> > Sure is, which is why we should be moving away from the legacy SR-IOV
> > APIs.  
> 
> Agreed!
> I do not suggest to extend/improve this API, just make sure it's not broken.
> 


  reply	other threads:[~2023-06-07 16:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-23  4:53 [PATCH net-next 0/4] support for 256 VFs in RTM_GETLINK Edwin Peer
2021-01-23  4:53 ` [PATCH net-next 1/4] netlink: truncate overlength attribute list in nla_nest_end() Edwin Peer
2021-01-23 19:14   ` David Ahern
2021-01-23 20:42     ` Edwin Peer
2021-01-23 21:03       ` Edwin Peer
2021-01-26  4:56         ` David Ahern
2021-01-26 17:51           ` Edwin Peer
2023-06-05  7:28             ` Gal Pressman
2023-06-05 18:58               ` Jakub Kicinski
2023-06-05 19:27                 ` Edwin Peer
2023-06-06  8:01                   ` Gal Pressman
2023-06-06 16:17                     ` Jakub Kicinski
2023-06-07 13:31                       ` Gal Pressman
2023-06-07 16:33                         ` Jakub Kicinski [this message]
2023-06-07 16:52                           ` Stephen Hemminger
2023-06-07 17:29                             ` Jakub Kicinski
2021-01-26  4:50       ` David Ahern
2021-01-26  1:43   ` Jakub Kicinski
2021-01-23  4:53 ` [PATCH net-next 2/4] rtnetlink: extend RTEXT_FILTER_SKIP_STATS to IFLA_VF_INFO Edwin Peer
2021-01-26  1:55   ` Jakub Kicinski
2021-01-26 22:48     ` Edwin Peer
2021-01-23  4:53 ` [PATCH net-next 3/4] rtnetlink: refactor IFLA_VF_INFO stats into rtnl_fill_vfstats() Edwin Peer
2021-01-23  4:53 ` [PATCH net-next 4/4] rtnetlink: promote IFLA_VF_STATS to same level as IFLA_VF_INFO Edwin Peer
2021-01-26  2:01   ` Jakub Kicinski
2021-01-26 14:50     ` Edwin Peer

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=20230607093324.2b7712d9@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew.gospodarek@broadcom.com \
    --cc=dsahern@gmail.com \
    --cc=espeer@gmail.com \
    --cc=gal@nvidia.com \
    --cc=michael.chan@broadcom.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).