public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top
Subject: Re: [PATCH] netlink: Use correct interface index in NL_SET mode
Date: Tue, 27 Jun 2023 19:18:23 +0200	[thread overview]
Message-ID: <20230627191823.053b8903@elisabeth> (raw)
In-Reply-To: <20230627102233.3114542-1-david@gibson.dropbear.id.au>

On Tue, 27 Jun 2023 20:22:33 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> nl_addr() and nl_route() take an 'op' selector which affects a number of
> parameters to the netlink call.  Unfortunately when we introduced this
> option a bug was introduced so that we always use the interface index for
> the host side, rather than the one for the pasta namespace.

Oops, right. Not so luckily, in the tests in my environments, as well
as in Podman's CI environment, interface indices actually match...

> Really, the entire interface to nl_addr() and nl_route() is pretty bad:
> it's tightly coupled with the use cases of its callers.

I wouldn't call that specifically a bad thing... with no users, it's,
strictly speaking, useless. What's worse in my opinion is the resulting
duplication (i.e. each function being specific to *one* caller).

I was considering to introduce in that same series a struct
representing possible configuration actions, including, say, enum
nl_conf { NL_CONF_ADDR, NL_CONF_ROUTE, ... }, but I realised it would
be kind of invasive, so I gave up for the moment.

> This is a minimal
> fix which doesn't address that, but also doesn't make it significantly
> worse.
> 
> Bugzilla: https://bugs.passt.top/show_bug.cgi?id=59
> Fixes: 2fe046185634 ("netlink: Add functionality to copy routes from outer namespace")
> Fixes: e89da3cf03b2 ("netlink: Add functionality to copy addresses from outer namespace")
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied, thanks.

-- 
Stefano


      reply	other threads:[~2023-06-27 17:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 10:22 [PATCH] netlink: Use correct interface index in NL_SET mode David Gibson
2023-06-27 17:18 ` Stefano Brivio [this message]

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=20230627191823.053b8903@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    /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.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).