From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55182) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLxyr-0006w2-GM for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:14:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLxyg-00055V-QS for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:13:53 -0400 Received: from mail-ee0-f45.google.com ([74.125.83.45]:41642) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLxyg-00055J-K1 for qemu-devel@nongnu.org; Wed, 10 Oct 2012 11:13:42 -0400 Received: by mail-ee0-f45.google.com with SMTP id b47so431344eek.4 for ; Wed, 10 Oct 2012 08:13:41 -0700 (PDT) Message-ID: <1349881520.2387.4.camel@moon.ceda.unina2.it> From: Mario De Chenno Date: Wed, 10 Oct 2012 17:05:20 +0200 Content-Type: multipart/alternative; boundary="=-5JPrk4XZXegkRP+4xNFB" Mime-Version: 1.0 Subject: [Qemu-devel] [feature request] qemu-bridge-helper Reply-To: mario.dechenno@unina2.it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org --=-5JPrk4XZXegkRP+4xNFB Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit It could be possible to add a sort of "ifname=" option to the qemu-bridge-helper program, so qemu could give a recognizable name to the tap interface the helper creates? Thanks all, Mairo De Chenno --=-5JPrk4XZXegkRP+4xNFB Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 7bit It could be possible to add a sort of "ifname=" option to the qemu-bridge-helper program, so qemu could give a recognizable name to the tap interface the helper creates?
Thanks  all,
Mairo De Chenno --=-5JPrk4XZXegkRP+4xNFB-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49402) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMKpx-0008F1-Ef for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:38:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMKpq-0005xV-2v for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:38:12 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:40652) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMKpp-0005wV-RM for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:38:06 -0400 Received: from /spool/local by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Oct 2012 09:38:00 -0600 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 4B09D3E4013D for ; Thu, 11 Oct 2012 09:31:17 -0600 (MDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q9BFVGr3180788 for ; Thu, 11 Oct 2012 09:31:16 -0600 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q9BFVEMO028861 for ; Thu, 11 Oct 2012 09:31:15 -0600 Message-ID: <5076E640.4090003@linux.vnet.ibm.com> Date: Thu, 11 Oct 2012 11:31:12 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1349881520.2387.4.camel@moon.ceda.unina2.it> In-Reply-To: <1349881520.2387.4.camel@moon.ceda.unina2.it> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [feature request] qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mario.dechenno@unina2.it, Richa Marwaha Cc: Mario De Chenno , qemu-devel@nongnu.org On 10/10/2012 11:05 AM, Mario De Chenno wrote: > It could be possible to add a sort of "ifname=" option to the > qemu-bridge-helper program, so qemu could give a recognizable name to > the tap interface the helper creates? > Thanks all, > Mairo De Chenno I don't see why not. If ifname is not specified then we can default to tap0, etc. Richa, do you want to handle this? -- Regards, Corey Bryant From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33313) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMZVr-0004vo-RY for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:18:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMZVq-0007fg-Ka for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:18:27 -0400 Received: from 69.169.164.127.provo.static.broadweavenetworks.net ([69.169.164.127]:37707 helo=baldr.dev-zero.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMZVq-0007fO-FM for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:18:26 -0400 Message-ID: <5077C0A7.4090500@dev-zero.net> Date: Fri, 12 Oct 2012 01:03:03 -0600 From: Mike Lovell MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> In-Reply-To: <1350024543-26211-1-git-send-email-mike@dev-zero.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: stefanha@gmail.com On 10/12/2012 12:49 AM, Mike Lovell wrote: > This makes a few changes to allow ifname to be specified when using > qemu-bridge-helper with both the bridge and tap network interfaces. It adds > the --ifname option to qemu-bridge-helper, removes the restriction that ifname > cannot be specified with helper for the tap interface, and adds logic to > specify the --ifname option when exec'ing the helper. > > Signed-off-by: Mike Lovell > --- > > This feature was originally requested by Mario De Chenno on the qemu-devel > mailing list. Seems pretty simple and figured it was something I could throw > together pretty quickly. I have tested the following combinations of invoking > qemu (where qbr is qemu-bridge-helper) > > qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1" > qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1",ifname=vm1 > qemu-system-x86_64 -net nic -net tap,helper=qbr > qemu-system-x86_64 -net nic -net tap,helper=qbr,ifname=vm1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr > qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1,br=test1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr,br=test1 I realized there were a couple more cases to check. I also tested the following. qemu-system-x86_64 -net nic -net tap,helper="qbr --ifname=vm1" qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1 --ifname=vm1" qemu-system-x86_64 -net nic -net tap,helper="qbr --ifname=vm1",ifname=foo qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1 --ifname=vm1",ifname=foo In the last two cases, the --ifname specified in the helper option (in this case, vm1) is used over the tap ifname option (in this case, foo). mike From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53778) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMZfc-0007EC-FV for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:28:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMZfW-0002a6-3W for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:28:32 -0400 Received: from 69.169.164.127.provo.static.broadweavenetworks.net ([69.169.164.127]:37709 helo=baldr.dev-zero.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMZfV-0002a2-SK for qemu-devel@nongnu.org; Fri, 12 Oct 2012 03:28:26 -0400 From: Mike Lovell Date: Fri, 12 Oct 2012 00:49:03 -0600 Message-Id: <1350024543-26211-1-git-send-email-mike@dev-zero.net> In-Reply-To: <5076E640.4090003@linux.vnet.ibm.com> References: <5076E640.4090003@linux.vnet.ibm.com> Subject: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: stefanha@gmail.com, Mike Lovell This makes a few changes to allow ifname to be specified when using qemu-bridge-helper with both the bridge and tap network interfaces. It adds the --ifname option to qemu-bridge-helper, removes the restriction that ifname cannot be specified with helper for the tap interface, and adds logic to specify the --ifname option when exec'ing the helper. Signed-off-by: Mike Lovell --- This feature was originally requested by Mario De Chenno on the qemu-devel mailing list. Seems pretty simple and figured it was something I could throw together pretty quickly. I have tested the following combinations of invoking qemu (where qbr is qemu-bridge-helper) qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1" qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1",ifname=vm1 qemu-system-x86_64 -net nic -net tap,helper=qbr qemu-system-x86_64 -net nic -net tap,helper=qbr,ifname=vm1 qemu-system-x86_64 -net nic -net bridge,helper=qbr qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1 qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1,br=test1 qemu-system-x86_64 -net nic -net bridge,helper=qbr,br=test1 net/tap.c | 39 ++++++++++++++++++++++++++++----------- qapi-schema.json | 3 ++- qemu-bridge-helper.c | 10 +++++++--- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/net/tap.c b/net/tap.c index a88ae8f..cfb5bff 100644 --- a/net/tap.c +++ b/net/tap.c @@ -417,11 +417,13 @@ static int recv_fd(int c) return len; } -static int net_bridge_run_helper(const char *helper, const char *bridge) +static int net_bridge_run_helper(const char *helper, + const char *bridge, + const char *ifname) { sigset_t oldmask, mask; int pid, status; - char *args[5]; + char *args[6]; char **parg; int sv[2]; @@ -439,7 +441,9 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) int open_max = sysconf(_SC_OPEN_MAX), i; char fd_buf[6+10]; char br_buf[6+IFNAMSIZ] = {0}; - char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15]; + char ifname_buf[10+IFNAMSIZ] = {0}; + char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + + sizeof(ifname_buf) + 15]; for (i = 0; i < open_max; i++) { if (i != STDIN_FILENO && @@ -459,8 +463,13 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge); } - snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s", - helper, "--use-vnet", fd_buf, br_buf); + if ((strstr(helper, "--ifname=") == NULL) && (ifname != NULL)) { + snprintf(ifname_buf, sizeof(ifname_buf), "%s%s" , + "--ifname=", ifname); + } + + snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s %s", + helper, "--use-vnet", fd_buf, br_buf, ifname_buf); parg = args; *parg++ = (char *)"sh"; @@ -473,12 +482,17 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) /* assume helper is just the executable path name */ snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge); + if (ifname != NULL) { + snprintf(ifname_buf, sizeof(ifname_buf), "%s%s" , + "--ifname=", ifname); + } parg = args; *parg++ = (char *)helper; *parg++ = (char *)"--use-vnet"; *parg++ = fd_buf; *parg++ = br_buf; + *parg++ = ifname_buf; *parg++ = NULL; execv(helper, args); @@ -517,7 +531,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, NetClientState *peer) { const NetdevBridgeOptions *bridge; - const char *helper, *br; + const char *helper, *br, *ifname; TAPState *s; int fd, vnet_hdr; @@ -527,8 +541,9 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER; br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE; + ifname = bridge->has_ifname ? bridge->ifname : NULL; - fd = net_bridge_run_helper(helper, br); + fd = net_bridge_run_helper(helper, br, ifname); if (fd == -1) { return -1; } @@ -622,14 +637,16 @@ int net_init_tap(const NetClientOptions *opts, const char *name, model = "tap"; } else if (tap->has_helper) { - if (tap->has_ifname || tap->has_script || tap->has_downscript || - tap->has_vnet_hdr) { - error_report("ifname=, script=, downscript=, and vnet_hdr= " + if (tap->has_script || tap->has_downscript || tap->has_vnet_hdr) { + error_report("script=, downscript=, and vnet_hdr= " "are invalid with helper="); return -1; } - fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE); + const char *ifname; + ifname = tap->has_ifname ? tap->ifname : NULL; + fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE, + ifname); if (fd == -1) { return -1; } diff --git a/qapi-schema.json b/qapi-schema.json index f9dbdae..feaac9e 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2432,7 +2432,8 @@ { 'type': 'NetdevBridgeOptions', 'data': { '*br': 'str', - '*helper': 'str' } } + '*helper': 'str', + '*ifname': 'str' } } ## # @NetdevHubPortOptions diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index 652eec9..c1d1519 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -67,7 +67,8 @@ typedef QSIMPLEQ_HEAD(ACLList, ACLRule) ACLList; static void usage(void) { fprintf(stderr, - "Usage: qemu-bridge-helper [--use-vnet] --br=bridge --fd=unixfd\n"); + "Usage: qemu-bridge-helper [--use-vnet] [--ifname=name] " + "--br=bridge --fd=unixfd\n"); } static int parse_acl_file(const char *filename, ACLList *acl_list) @@ -239,6 +240,7 @@ int main(int argc, char **argv) ACLList acl_list; int access_allowed, access_denied; int ret = EXIT_SUCCESS; + const char *ifname = NULL; #ifdef CONFIG_LIBCAP /* if we're run from an suid binary, immediately drop privileges preserving @@ -259,6 +261,8 @@ int main(int argc, char **argv) bridge = &argv[index][5]; } else if (strncmp(argv[index], "--fd=", 5) == 0) { unixfd = atoi(&argv[index][5]); + } else if (strncmp(argv[index], "--ifname=", 9) == 0) { + ifname = &argv[index][9]; } else { usage(); return EXIT_FAILURE; @@ -329,8 +333,8 @@ int main(int argc, char **argv) } /* request a tap device, disable PI, and add vnet header support if - * requested and it's available. */ - prep_ifreq(&ifr, "tap%d"); + * requested and it's available. use ifname if provided for tap name. */ + prep_ifreq(&ifr, ifname != NULL ? ifname : "tap%d"); ifr.ifr_flags = IFF_TAP|IFF_NO_PI; if (use_vnet && has_vnet_hdr(fd)) { ifr.ifr_flags |= IFF_VNET_HDR; -- 1.7.9.5 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMafH-0006ES-4c for qemu-devel@nongnu.org; Fri, 12 Oct 2012 04:32:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMafG-0008Q3-2G for qemu-devel@nongnu.org; Fri, 12 Oct 2012 04:32:15 -0400 Received: from isrv.corpit.ru ([86.62.121.231]:56844) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMafF-0008O0-R4 for qemu-devel@nongnu.org; Fri, 12 Oct 2012 04:32:13 -0400 Message-ID: <5077D584.3040204@msgid.tls.msk.ru> Date: Fri, 12 Oct 2012 12:32:04 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> In-Reply-To: <1350024543-26211-1-git-send-email-mike@dev-zero.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Lovell Cc: stefanha@gmail.com, qemu-devel@nongnu.org On 12.10.2012 10:49, Mike Lovell wrote: > /* request a tap device, disable PI, and add vnet header support if > - * requested and it's available. */ > - prep_ifreq(&ifr, "tap%d"); > + * requested and it's available. use ifname if provided for tap name. */ > + prep_ifreq(&ifr, ifname != NULL ? ifname : "tap%d"); Should we check for special symbols here? prep_ifreq() does this: snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname); so at least it ensures we have length constraint. Actually I'm not so sure anymore this is a good idea. For example, system may have firewall (iptables) rules in place for, say, future ppp interfaces for ppp clients, and this way we may request the interface to be named pppX and be allowed to send packets where we don't usually have access to. Maybe - at least - require some common prefix for the interfaces created this way, so we'll live in our own, easily distinguishable namespace -- like, qvif* (from Qemu Virtual InterFace)? This is not a simple question really. And the whole bridge helper is quite questionable too. Thanks, /mjt From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57216) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMjaN-00052S-O0 for qemu-devel@nongnu.org; Fri, 12 Oct 2012 14:03:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMjaM-0007BY-Rt for qemu-devel@nongnu.org; Fri, 12 Oct 2012 14:03:47 -0400 Received: from 69.169.164.127.provo.static.broadweavenetworks.net ([69.169.164.127]:37769 helo=baldr.dev-zero.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMjaM-0007BU-Mc for qemu-devel@nongnu.org; Fri, 12 Oct 2012 14:03:46 -0400 Message-ID: <50785B9F.50609@dev-zero.net> Date: Fri, 12 Oct 2012 12:04:15 -0600 From: Mike Lovell MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> <5077D584.3040204@msgid.tls.msk.ru> In-Reply-To: <5077D584.3040204@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-devel@nongnu.org On 10/12/2012 02:32 AM, Michael Tokarev wrote: > On 12.10.2012 10:49, Mike Lovell wrote: >> /* request a tap device, disable PI, and add vnet header support if >> - * requested and it's available. */ >> - prep_ifreq(&ifr, "tap%d"); >> + * requested and it's available. use ifname if provided for tap name. */ >> + prep_ifreq(&ifr, ifname != NULL ? ifname : "tap%d"); > Should we check for special symbols here? prep_ifreq() does this: > > snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname); > > so at least it ensures we have length constraint. I tried the code as is with specifying ifnames with various random combinations of special characters. Some of them we just allowed through, some caused an error when initializing the tap device, and some cause problems in the shell invoking qemu. I think the linux kernel does the necessary checking during the TUNSETIFF ioctl and the qemu-bridge-helper exits with an error if there was a problem. > Actually I'm not so sure anymore this is a good idea. > For example, system may have firewall (iptables) rules > in place for, say, future ppp interfaces for ppp clients, > and this way we may request the interface to be named > pppX and be allowed to send packets where we don't usually > have access to. While I admit this does have that possibility, I'm not sure its a qemu problem. I don't know what the original motivation for the request was but I could see this being the reason for the request. Some administrator sets up firewall rules for a variety of guests ahead of actually running them and needs to specify the interface at runtime. Also, without using the helper programs, the qemu already allows specifying arbitrary names such as ppp0. > Maybe - at least - require some common prefix for the > interfaces created this way, so we'll live in our own, > easily distinguishable namespace -- like, qvif* (from > Qemu Virtual InterFace)? I do like the idea of using a common prefix for the default name of tap devices. Something like "qvif%d" instead of "tap%d" in tap initialization code. But something tells me this could break compatibility with external management software where something might be expecting the interface name to start with tap. mike From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37180) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeKjz-0004KP-9t for qemu-devel@nongnu.org; Fri, 30 Nov 2012 02:10:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TeKjx-0007VZ-Gr for qemu-devel@nongnu.org; Fri, 30 Nov 2012 02:10:27 -0500 Received: from 69.169.164.127.provo.static.broadweavenetworks.net ([69.169.164.127]:41451 helo=baldr.dev-zero.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeKjx-0007VV-8O for qemu-devel@nongnu.org; Fri, 30 Nov 2012 02:10:25 -0500 Message-ID: <50B85BEA.6000107@dev-zero.net> Date: Fri, 30 Nov 2012 00:10:34 -0700 From: Mike Lovell MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> In-Reply-To: <1350024543-26211-1-git-send-email-mike@dev-zero.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, stefanha@gmail.com On 10/12/2012 12:49 AM, Mike Lovell wrote: > This makes a few changes to allow ifname to be specified when using > qemu-bridge-helper with both the bridge and tap network interfaces. It adds > the --ifname option to qemu-bridge-helper, removes the restriction that ifname > cannot be specified with helper for the tap interface, and adds logic to > specify the --ifname option when exec'ing the helper. > > Signed-off-by: Mike Lovell > --- > > This feature was originally requested by Mario De Chenno on the qemu-devel > mailing list. Seems pretty simple and figured it was something I could throw > together pretty quickly. I have tested the following combinations of invoking > qemu (where qbr is qemu-bridge-helper) > > qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1" > qemu-system-x86_64 -net nic -net tap,helper="qbr --br=test1",ifname=vm1 > qemu-system-x86_64 -net nic -net tap,helper=qbr > qemu-system-x86_64 -net nic -net tap,helper=qbr,ifname=vm1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr > qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr,ifname=vm1,br=test1 > qemu-system-x86_64 -net nic -net bridge,helper=qbr,br=test1 > > net/tap.c | 39 ++++++++++++++++++++++++++++----------- > qapi-schema.json | 3 ++- > qemu-bridge-helper.c | 10 +++++++--- > 3 files changed, 37 insertions(+), 15 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index a88ae8f..cfb5bff 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -417,11 +417,13 @@ static int recv_fd(int c) > return len; > } > > -static int net_bridge_run_helper(const char *helper, const char *bridge) > +static int net_bridge_run_helper(const char *helper, > + const char *bridge, > + const char *ifname) > { > sigset_t oldmask, mask; > int pid, status; > - char *args[5]; > + char *args[6]; > char **parg; > int sv[2]; > > @@ -439,7 +441,9 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) > int open_max = sysconf(_SC_OPEN_MAX), i; > char fd_buf[6+10]; > char br_buf[6+IFNAMSIZ] = {0}; > - char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15]; > + char ifname_buf[10+IFNAMSIZ] = {0}; > + char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + > + sizeof(ifname_buf) + 15]; > > for (i = 0; i < open_max; i++) { > if (i != STDIN_FILENO && > @@ -459,8 +463,13 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) > snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge); > } > > - snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s", > - helper, "--use-vnet", fd_buf, br_buf); > + if ((strstr(helper, "--ifname=") == NULL) && (ifname != NULL)) { > + snprintf(ifname_buf, sizeof(ifname_buf), "%s%s" , > + "--ifname=", ifname); > + } > + > + snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s %s", > + helper, "--use-vnet", fd_buf, br_buf, ifname_buf); > > parg = args; > *parg++ = (char *)"sh"; > @@ -473,12 +482,17 @@ static int net_bridge_run_helper(const char *helper, const char *bridge) > /* assume helper is just the executable path name */ > > snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge); > + if (ifname != NULL) { > + snprintf(ifname_buf, sizeof(ifname_buf), "%s%s" , > + "--ifname=", ifname); > + } > > parg = args; > *parg++ = (char *)helper; > *parg++ = (char *)"--use-vnet"; > *parg++ = fd_buf; > *parg++ = br_buf; > + *parg++ = ifname_buf; > *parg++ = NULL; > > execv(helper, args); > @@ -517,7 +531,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, > NetClientState *peer) > { > const NetdevBridgeOptions *bridge; > - const char *helper, *br; > + const char *helper, *br, *ifname; > > TAPState *s; > int fd, vnet_hdr; > @@ -527,8 +541,9 @@ int net_init_bridge(const NetClientOptions *opts, const char *name, > > helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER; > br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE; > + ifname = bridge->has_ifname ? bridge->ifname : NULL; > > - fd = net_bridge_run_helper(helper, br); > + fd = net_bridge_run_helper(helper, br, ifname); > if (fd == -1) { > return -1; > } > @@ -622,14 +637,16 @@ int net_init_tap(const NetClientOptions *opts, const char *name, > model = "tap"; > > } else if (tap->has_helper) { > - if (tap->has_ifname || tap->has_script || tap->has_downscript || > - tap->has_vnet_hdr) { > - error_report("ifname=, script=, downscript=, and vnet_hdr= " > + if (tap->has_script || tap->has_downscript || tap->has_vnet_hdr) { > + error_report("script=, downscript=, and vnet_hdr= " > "are invalid with helper="); > return -1; > } > > - fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE); > + const char *ifname; > + ifname = tap->has_ifname ? tap->ifname : NULL; > + fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE, > + ifname); > if (fd == -1) { > return -1; > } > diff --git a/qapi-schema.json b/qapi-schema.json > index f9dbdae..feaac9e 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2432,7 +2432,8 @@ > { 'type': 'NetdevBridgeOptions', > 'data': { > '*br': 'str', > - '*helper': 'str' } } > + '*helper': 'str', > + '*ifname': 'str' } } > > ## > # @NetdevHubPortOptions > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c > index 652eec9..c1d1519 100644 > --- a/qemu-bridge-helper.c > +++ b/qemu-bridge-helper.c > @@ -67,7 +67,8 @@ typedef QSIMPLEQ_HEAD(ACLList, ACLRule) ACLList; > static void usage(void) > { > fprintf(stderr, > - "Usage: qemu-bridge-helper [--use-vnet] --br=bridge --fd=unixfd\n"); > + "Usage: qemu-bridge-helper [--use-vnet] [--ifname=name] " > + "--br=bridge --fd=unixfd\n"); > } > > static int parse_acl_file(const char *filename, ACLList *acl_list) > @@ -239,6 +240,7 @@ int main(int argc, char **argv) > ACLList acl_list; > int access_allowed, access_denied; > int ret = EXIT_SUCCESS; > + const char *ifname = NULL; > > #ifdef CONFIG_LIBCAP > /* if we're run from an suid binary, immediately drop privileges preserving > @@ -259,6 +261,8 @@ int main(int argc, char **argv) > bridge = &argv[index][5]; > } else if (strncmp(argv[index], "--fd=", 5) == 0) { > unixfd = atoi(&argv[index][5]); > + } else if (strncmp(argv[index], "--ifname=", 9) == 0) { > + ifname = &argv[index][9]; > } else { > usage(); > return EXIT_FAILURE; > @@ -329,8 +333,8 @@ int main(int argc, char **argv) > } > > /* request a tap device, disable PI, and add vnet header support if > - * requested and it's available. */ > - prep_ifreq(&ifr, "tap%d"); > + * requested and it's available. use ifname if provided for tap name. */ > + prep_ifreq(&ifr, ifname != NULL ? ifname : "tap%d"); > ifr.ifr_flags = IFF_TAP|IFF_NO_PI; > if (use_vnet && has_vnet_hdr(fd)) { > ifr.ifr_flags |= IFF_VNET_HDR; ping ... or syn. any other thoughts about this? mike From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeNQm-0007Hn-On for qemu-devel@nongnu.org; Fri, 30 Nov 2012 05:02:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TeNQi-0003WR-Ga for qemu-devel@nongnu.org; Fri, 30 Nov 2012 05:02:48 -0500 Received: from isrv.corpit.ru ([86.62.121.231]:55032) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeNQi-0003L4-3f for qemu-devel@nongnu.org; Fri, 30 Nov 2012 05:02:44 -0500 Message-ID: <50B88437.2080204@msgid.tls.msk.ru> Date: Fri, 30 Nov 2012 14:02:31 +0400 From: Michael Tokarev MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> <5077D584.3040204@msgid.tls.msk.ru> <50785B9F.50609@dev-zero.net> In-Reply-To: <50785B9F.50609@dev-zero.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Lovell Cc: qemu-devel@nongnu.org Somehow I missed this email initially.. replying now. On 12.10.2012 22:04, Mike Lovell wrote: > On 10/12/2012 02:32 AM, Michael Tokarev wrote: >> On 12.10.2012 10:49, Mike Lovell wrote: >>> /* request a tap device, disable PI, and add vnet header support if >>> - * requested and it's available. */ >>> - prep_ifreq(&ifr, "tap%d"); >>> + * requested and it's available. use ifname if provided for tap name. */ >>> + prep_ifreq(&ifr, ifname != NULL ? ifname : "tap%d"); >> Should we check for special symbols here? prep_ifreq() does this: >> >> snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname); >> >> so at least it ensures we have length constraint. > > I tried the code as is with specifying ifnames with various random combinations of special characters. Some of them we just allowed through, some caused an error when initializing the tap device, and some cause problems in the shell invoking qemu. I think the linux kernel does the necessary checking during the TUNSETIFF ioctl and the qemu-bridge-helper exits with an error if there was a problem. > >> Actually I'm not so sure anymore this is a good idea. >> For example, system may have firewall (iptables) rules >> in place for, say, future ppp interfaces for ppp clients, >> and this way we may request the interface to be named >> pppX and be allowed to send packets where we don't usually >> have access to. > > While I admit this does have that possibility, I'm not sure its a qemu problem. I don't know what the original motivation for the request was but I could see this being the reason for the request. Some administrator sets up firewall rules for a variety of guests ahead of actually running them and needs to specify the interface at runtime. Also, without using the helper programs, the qemu already allows specifying arbitrary names such as ppp0. qemu allows arbitrary names, yes, but it does not have extra permissions to create them, -- only ones of the current user. The helper, on the other hand, does have extra privileges which a regular user does not. That's exactly what I was talking about. Maybe _always_ having a common prefix is a good idea after all, with --name=FOO appended to it, like qvifFOO. Or use --ifnumber=NNN instead of --name (which I dislike). >> Maybe - at least - require some common prefix for the >> interfaces created this way, so we'll live in our own, >> easily distinguishable namespace -- like, qvif* (from >> Qemu Virtual InterFace)? > > I do like the idea of using a common prefix for the default name of tap devices. Something like "qvif%d" instead of "tap%d" in tap initialization code. But something tells me this could break compatibility with external management software where something might be expecting the interface name to start with tap. Does any management interface use this bridge-helper functionality? If it were me, I'd always created the tap fd in the management layer and passed the tap fd# (or at least ifname= of an existing iface) to qemu. Bridge helper is useful for users calling qemu directly, not for management software. Sure, such users are also important - including compatibility. But I don't think current unpredictable tapNN names was a good idea to start with, and that it's good idea to rely on this prefix in firewall rules or whatnot. Thanks, /mjt From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42258) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeRdg-0001oE-55 for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:32:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TeRda-0003iZ-CU for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:32:24 -0500 Received: from mail-pb0-f45.google.com ([209.85.160.45]:42872) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeRda-0003iR-5w for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:32:18 -0500 Received: by mail-pb0-f45.google.com with SMTP id mc8so452356pbc.4 for ; Fri, 30 Nov 2012 06:32:16 -0800 (PST) Sender: Paolo Bonzini Message-ID: <50B8C368.6010500@redhat.com> Date: Fri, 30 Nov 2012 15:32:08 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> <5077D584.3040204@msgid.tls.msk.ru> <50785B9F.50609@dev-zero.net> <50B88437.2080204@msgid.tls.msk.ru> In-Reply-To: <50B88437.2080204@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: Mike Lovell , qemu-devel@nongnu.org Il 30/11/2012 11:02, Michael Tokarev ha scritto: >> I do like the idea of using a common prefix for the default name >> of tap devices. Something like "qvif%d" instead of "tap%d" in tap >> initialization code. But something tells me this could break >> compatibility with external management software where something >> might be expecting the interface name to start with tap. > > Does any management interface use this bridge-helper functionality? Libvirt uses it if you're running libvirtd without privileges. GNOME Boxes is a 1-line patch away from using it, but it's Fedora-specific and not included in any distro. Paolo > If it were me, I'd always created the tap fd in the management layer > and passed the tap fd# (or at least ifname= of an existing iface) to > qemu. Bridge helper is useful for users calling qemu directly, not > for management software. Sure, such users are also important - > including compatibility. But I don't think current unpredictable > tapNN names was a good idea to start with, and that it's good idea to > rely on this prefix in firewall rules or whatnot. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeRhA-0002ke-SR for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:36:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TeRh4-0004nY-N8 for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:36:00 -0500 Received: from mail-pa0-f45.google.com ([209.85.220.45]:36503) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TeRh4-0004nS-Gv for qemu-devel@nongnu.org; Fri, 30 Nov 2012 09:35:54 -0500 Received: by mail-pa0-f45.google.com with SMTP id bg2so367249pad.4 for ; Fri, 30 Nov 2012 06:35:53 -0800 (PST) Sender: Paolo Bonzini Message-ID: <50B8C442.70007@redhat.com> Date: Fri, 30 Nov 2012 15:35:46 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> <50B85BEA.6000107@dev-zero.net> In-Reply-To: <50B85BEA.6000107@dev-zero.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mike Lovell Cc: stefanha@gmail.com, qemu-devel@nongnu.org Il 30/11/2012 08:10, Mike Lovell ha scritto: > On 10/12/2012 12:49 AM, Mike Lovell wrote: >> This makes a few changes to allow ifname to be specified when using >> qemu-bridge-helper with both the bridge and tap network interfaces. It >> adds >> the --ifname option to qemu-bridge-helper, removes the restriction >> that ifname >> cannot be specified with helper for the tap interface, and adds logic to >> specify the --ifname option when exec'ing the helper. > > ping ... or syn. any other thoughts about this? I share Michael's perplexity. This feature could be exploitable. If we want to add this, the ifname should be subject to ACL rules just like bridge names. For example you could have a special allow/deny directive "allow foo@" which allows ifnames starting with "foo". Paolo From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49860) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TfVmz-0001UE-L2 for qemu-devel@nongnu.org; Mon, 03 Dec 2012 08:10:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TfVmv-0005CU-L7 for qemu-devel@nongnu.org; Mon, 03 Dec 2012 08:10:25 -0500 Received: from mail-ee0-f45.google.com ([74.125.83.45]:59083) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TfVmv-0005CG-Ei for qemu-devel@nongnu.org; Mon, 03 Dec 2012 08:10:21 -0500 Received: by mail-ee0-f45.google.com with SMTP id d49so1691950eek.4 for ; Mon, 03 Dec 2012 05:10:20 -0800 (PST) Date: Mon, 3 Dec 2012 14:10:18 +0100 From: Stefan Hajnoczi Message-ID: <20121203131018.GA12912@stefanha-thinkpad.redhat.com> References: <5076E640.4090003@linux.vnet.ibm.com> <1350024543-26211-1-git-send-email-mike@dev-zero.net> <50B85BEA.6000107@dev-zero.net> <50B8C442.70007@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50B8C442.70007@redhat.com> Subject: Re: [Qemu-devel] [PATCH] net: Allow specifying ifname for qemu-bridge-helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Mike Lovell , qemu-devel@nongnu.org On Fri, Nov 30, 2012 at 03:35:46PM +0100, Paolo Bonzini wrote: > Il 30/11/2012 08:10, Mike Lovell ha scritto: > > On 10/12/2012 12:49 AM, Mike Lovell wrote: > >> This makes a few changes to allow ifname to be specified when using > >> qemu-bridge-helper with both the bridge and tap network interfaces. It > >> adds > >> the --ifname option to qemu-bridge-helper, removes the restriction > >> that ifname > >> cannot be specified with helper for the tap interface, and adds logic to > >> specify the --ifname option when exec'ing the helper. > > > > ping ... or syn. any other thoughts about this? > > I share Michael's perplexity. This feature could be exploitable. > > If we want to add this, the ifname should be subject to ACL rules just > like bridge names. For example you could have a special allow/deny > directive "allow foo@" which allows ifnames starting with "foo". This is a good idea. The default should be that you are not allowed to choose arbitrary interface names. Stefan