From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 7B8645A005E for ; Thu, 17 Nov 2022 16:33:14 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668699193; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=oxZRGJjmDColBg1LTXQ7UU6CvE/ZWlr4U82hcEbooog=; b=Wb564QEDCCMGECjuSp2zqXKBc7JhIOTXlqbk77KQDy0oHop/bcXItelHHBH/0lwPptbTb/ 0yjr9Z7hqFdB0rlMLcDwGoFVaPHrfz1eqIXRBm1pHo2JYYCoXsTwkcP/ZGOY/klezp5078 ifXyLejpwarvqzB1Zi4yVx9BPgB2J74= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-313-PBounAFVN7aJL2VnSdtN1A-1; Thu, 17 Nov 2022 10:33:09 -0500 X-MC-Unique: PBounAFVN7aJL2VnSdtN1A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7EEC838173D5 for ; Thu, 17 Nov 2022 15:33:09 +0000 (UTC) Received: from localhost (unknown [10.39.193.198]) by smtp.corp.redhat.com (Postfix) with ESMTP id 415BC111E3ED; Thu, 17 Nov 2022 15:33:09 +0000 (UTC) Date: Thu, 17 Nov 2022 15:33:06 +0000 From: "Richard W.M. Jones" To: Stefano Brivio Subject: Re: [PATCH v2 3/5] passt, tap: Add --fd option Message-ID: <20221117153306.GF7636@redhat.com> References: <20221117122614.1269214-4-rjones@redhat.com> <20221117152640.2535159-1-sbrivio@redhat.com> MIME-Version: 1.0 In-Reply-To: <20221117152640.2535159-1-sbrivio@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Message-ID-Hash: Y77SWKJQCQM4USO7HMEK33HZHIX5LQDT X-Message-ID-Hash: Y77SWKJQCQM4USO7HMEK33HZHIX5LQDT X-MailFrom: rjones@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, Nov 17, 2022 at 04:26:40PM +0100, Stefano Brivio wrote: > From: "Richard W.M. Jones" > > This passes a fully connected stream socket to passt. > > Signed-off-by: Richard W.M. Jones > [sbrivio: reuse fd_tap instead of adding a new descriptor, > imply --one-off on --fd, add to optstring and usage()] > Signed-off-by: Stefano Brivio > --- > v2: > - reuse fd_tap, we don't need a separate file descriptor > - add F to optstring and usage(), for both passt and pasta > - imply --one-off, we can't do much once the socket is closed > > With this, the trick from 5/5 goes a bit further: passt reads > from the file descriptor passed by the wrapper. Thanks for the v2 .. I'll add it to my series and play with it. > However, we get EPOLLRDHUP right away, from the close() on > one end of the socket pair I guess. Should we just ignore > all EPOLLRDHUP events, just the first one...? Does it see the event out-of-band or does it get an in-band read(2) == 0 after finishing reading the data? Ideally what should happen is that passt reads and parses all or as much of the data as it can, and then it responds to the closed socket by exiting. The reason for this is so that we have as much opportunity as possible to break the parser and crash passt. If passt was exiting before fully reading/parsing everything that it could, then we wouldn't be fuzzing the parser as deeply as we can. Note that the fuzzer will (quite routinely and normally) create test cases which are total nonsense, eg. packets with incorrect length field, truncated final packet, etc. Rich. > conf.c | 28 ++++++++++++++++++++++++++-- > passt.1 | 10 ++++++++++ > passt.c | 1 - > passt.h | 2 +- > tap.c | 9 +++++++++ > 5 files changed, 46 insertions(+), 4 deletions(-) > > diff --git a/conf.c b/conf.c > index a995eb7..2c374bd 100644 > --- a/conf.c > +++ b/conf.c > @@ -719,6 +719,7 @@ static void usage(const char *name) > UNIX_SOCK_PATH, 1); > } > > + info( " -F, --fd FD Use FD as pre-opened connected socket"); > info( " -p, --pcap FILE Log tap-facing traffic to pcap file"); > info( " -P, --pid FILE Write own PID to the given file"); > info( " -m, --mtu MTU Assign MTU via DHCP/NDP"); > @@ -1079,6 +1080,7 @@ void conf(struct ctx *c, int argc, char **argv) > {"log-file", required_argument, NULL, 'l' }, > {"help", no_argument, NULL, 'h' }, > {"socket", required_argument, NULL, 's' }, > + {"fd", required_argument, NULL, 'F' }, > {"ns-ifname", required_argument, NULL, 'I' }, > {"pcap", required_argument, NULL, 'p' }, > {"pid", required_argument, NULL, 'P' }, > @@ -1138,9 +1140,9 @@ void conf(struct ctx *c, int argc, char **argv) > > if (c->mode == MODE_PASTA) { > c->no_dhcp_dns = c->no_dhcp_dns_search = 1; > - optstring = "dqfel:hI:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:"; > + optstring = "dqfel:hf:I:p:P:m:a:n:M:g:i:D:S:46t:u:T:U:"; > } else { > - optstring = "dqfel:hs:p:P:m:a:n:M:g:i:D:S:461t:u:"; > + optstring = "dqfel:hs:f:p:P:m:a:n:M:g:i:D:S:461t:u:"; > } > > c->tcp.fwd_in.mode = c->tcp.fwd_out.mode = 0; > @@ -1355,6 +1357,23 @@ void conf(struct ctx *c, int argc, char **argv) > err("Invalid socket path: %s", optarg); > usage(argv[0]); > } > + break; > + case 'F': > + if (c->fd_tap >= 0) { > + err("Multiple --fd options given"); > + usage(argv[0]); > + } > + > + errno = 0; > + c->fd_tap = strtol(optarg, NULL, 0); > + > + if (c->fd_tap < 0 || errno) { > + err("Invalid --fd: %s", optarg); > + usage(argv[0]); > + } > + > + c->one_off = true; > + > break; > case 'I': > if (*c->pasta_ifn) { > @@ -1590,6 +1609,11 @@ void conf(struct ctx *c, int argc, char **argv) > usage(argv[0]); > } > > + if (*c->sock_path && c->fd_tap >= 0) { > + err("Options --socket and --fd are mutually exclusive"); > + usage(argv[0]); > + } > + > ret = conf_ugid(runas, &uid, &gid); > if (ret) > usage(argv[0]); > diff --git a/passt.1 b/passt.1 > index e34a3e0..528763b 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -297,6 +297,16 @@ Path for UNIX domain socket used by \fBqemu\fR(1) or \fBqrap\fR(1) to connect to > Default is to probe a free socket, not accepting connections, starting from > \fI/tmp/passt_1.socket\fR to \fI/tmp/passt_64.socket\fR. > > +.TP > +.BR \-F ", " \-\-fd " " \fIFD > +Pass a pre-opened, connected socket to \fBpasst\fR. Usually the socket is opened > +in the parent process and \fBpasst\fR inherits it when run as a child. This > +allows the parent process to open sockets using another address family or > +requiring special privileges. > + > +This option implies the behaviour described for \-\-one-off, once this socket > +is closed. > + > .TP > .BR \-1 ", " \-\-one-off > Quit after handling a single client connection, that is, once the client closes > diff --git a/passt.c b/passt.c > index 7d323c2..8b2c50d 100644 > --- a/passt.c > +++ b/passt.c > @@ -255,7 +255,6 @@ int main(int argc, char **argv) > > quit_fd = pasta_netns_quit_init(&c); > > - c.fd_tap = c.fd_tap_listen = -1; > tap_sock_init(&c); > > clock_gettime(CLOCK_MONOTONIC, &now); > diff --git a/passt.h b/passt.h > index 6649c0a..ca25b90 100644 > --- a/passt.h > +++ b/passt.h > @@ -159,7 +159,7 @@ struct ip6_ctx { > * @proc_net_udp: Stored handles for /proc/net/udp{,6} in init and ns > * @epollfd: File descriptor for epoll instance > * @fd_tap_listen: File descriptor for listening AF_UNIX socket, if any > - * @fd_tap: File descriptor for AF_UNIX socket or tuntap device > + * @fd_tap: AF_UNIX socket, tuntap device, or pre-opened socket > * @mac: Host MAC address > * @mac_guest: MAC address of guest or namespace, seen or configured > * @ifi4: Index of routable interface for IPv4, 0 if IPv4 disabled > diff --git a/tap.c b/tap.c > index d26af58..9998127 100644 > --- a/tap.c > +++ b/tap.c > @@ -1069,6 +1069,15 @@ void tap_sock_init(struct ctx *c) > } > > if (c->fd_tap != -1) { > + if (c->one_off) { /* Passed as --fd */ > + struct epoll_event ev = { 0 }; > + > + ev.data.fd = c->fd_tap; > + ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP; > + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, c->fd_tap, &ev); > + return; > + } > + > epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL); > close(c->fd_tap); > c->fd_tap = -1; > -- > 2.35.1 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html