netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: Petr Machata <petrm@nvidia.com>
Cc: Hangbin Liu <liuhangbin@gmail.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	 Shuah Khan <shuah@kernel.org>, David Ahern <dsahern@kernel.org>,
	linux-kselftest@vger.kernel.org,
	 Po-Hsu Lin <po-hsu.lin@canonical.com>,
	Guillaume Nault <gnault@redhat.com>,
	James Prestwood <prestwoj@gmail.com>,
	Jaehee Park <jhpark1013@gmail.com>,
	Ido Schimmel <idosch@nvidia.com>,
	Justin Iurman <justin.iurman@uliege.be>,
	Xin Long <lucien.xin@gmail.com>,
	James Chapman <jchapman@katalix.com>
Subject: Re: [PATCHv3 net-next 01/14] selftests/net: add lib.sh
Date: Wed, 06 Dec 2023 16:13:59 +0100	[thread overview]
Message-ID: <70497ad83be1c7bd715abc8f29c72ee39a381f58.camel@redhat.com> (raw)
In-Reply-To: <87jzpr5x0r.fsf@nvidia.com>

On Wed, 2023-12-06 at 13:32 +0100, Petr Machata wrote:
> Paolo Abeni <pabeni@redhat.com> writes:
> 
> > Side note for a possible follow-up: if you maintain $ns_list as global
> > variable, and remove from such list the ns deleted by cleanup_ns, you
> > could remove the cleanup trap from the individual test with something
> > alike:
> > 
> > final_cleanup_ns()
> > {
> > 	cleanup_ns $ns_list
> > }
> > 
> > trap final_cleanup_ns EXIT
> > 
> > No respin needed for the above, could be a follow-up if agreed upon.
> 
> If you propose this for the library then I'm against it. The exit trap
> is a global resource that the client scripts sometimes need to use as
> well, to do topology teardowns or just general cleanups. 
> So either the library would have to provide APIs for cleanup management, or the trap
> is for exclusive use by clients. The latter is IMHO simpler.

Even the former would not be very complex:

TRAPS=""
do_at_exit() {
        TRAPS="${TRAPS}$@;"

        trap "${TRAPS}" EXIT
}

And then use "do_at_exit <whatever>" instead of "trap <whatever> EXIT"

> It also puts the cleanups at the same place where the acquisition is
> prompted: the client allocates the NS, the client should prompt its
> cleanup.

I guess I could argue that the the script is asking the library to
allocate the namespaces, and the library could take care of disposing
them.

But I'm not pushing the proposed option, if there is no agreement no
need for additional work ;)

Cheers,

Paolo


  reply	other threads:[~2023-12-06 15:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-02  2:00 [PATCHv3 net-next 00/14] Conver net selftests to run in unique namespace (Part 1) Hangbin Liu
2023-12-02  2:00 ` [PATCHv3 net-next 01/14] selftests/net: add lib.sh Hangbin Liu
2023-12-05 12:00   ` Paolo Abeni
2023-12-06  2:48     ` Hangbin Liu
2023-12-06 12:32     ` Petr Machata
2023-12-06 15:13       ` Paolo Abeni [this message]
2023-12-07 10:34         ` Petr Machata
2023-12-07 15:26           ` David Ahern
2023-12-02  2:00 ` [PATCHv3 net-next 02/14] selftests/net: convert arp_ndisc_evict_nocarrier.sh to run it in unique namespace Hangbin Liu
2023-12-02  2:00 ` [PATCHv3 net-next 03/14] selftests/net: specify the interface when do arping Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 04/14] selftests/net: convert arp_ndisc_untracked_subnets.sh to run it in unique namespace Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 05/14] selftests/net: convert cmsg tests to make them run " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 06/14] selftests/net: convert drop_monitor_tests.sh to run it " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 07/14] selftests/net: convert traceroute.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 08/14] selftests/net: convert icmp_redirect.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 09/14] sleftests/net: convert icmp.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 10/14] selftests/net: convert ioam6.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 11/14] selftests/net: convert l2tp.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 12/14] selftests/net: convert ndisc_unsolicited_na_test.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 13/14] selftests/net: convert sctp_vrf.sh " Hangbin Liu
2023-12-02  2:01 ` [PATCHv3 net-next 14/14] selftests/net: convert unicast_extensions.sh " Hangbin Liu
2023-12-05 12:30 ` [PATCHv3 net-next 00/14] Conver net selftests to run in unique namespace (Part 1) 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=70497ad83be1c7bd715abc8f29c72ee39a381f58.camel@redhat.com \
    --to=pabeni@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=gnault@redhat.com \
    --cc=idosch@nvidia.com \
    --cc=jchapman@katalix.com \
    --cc=jhpark1013@gmail.com \
    --cc=justin.iurman@uliege.be \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=lucien.xin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=petrm@nvidia.com \
    --cc=po-hsu.lin@canonical.com \
    --cc=prestwoj@gmail.com \
    --cc=shuah@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 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).