Various net and slirp fixes & enhancements
cover Various net and slirp fixes & enhancements
This is mostly a reposting of what I've already sent as individual patches. The only differences are the new patch 7 and the reworked patch 8 (reused of existing net client type names).
The network subsystem maintenance looks vacant, so I'm unsure who to address directly.
Jan Kiszka (9):
slirp: Fix restricted mode
slirp: Canonicalize restrict syntax
slirp: Strictly associate DHCP/BOOTP and TFTP with virtual host
slirp: Replace m_freem with m_free
slirp: Put forked exec into separate process group
net: Improve layout of 'info network'
net: Refactor net_client_types
net: Dump client type 'info network'
net: Consistently use qemu_macaddr_default_if_unset
hw/dp8393x.c | 2 +- hw/etraxfs_eth.c | 2 +- hw/mcf_fec.c | 2 +- hw/mipsnet.c | 2 +- hw/qdev.c | 2 +- hw/stellaris.c | 2 +- hw/xen_devconfig.c | 4 +- net.c | 65 +++++++++++++++++++++++++++++++-------------------- net.h | 8 ++++-- net/slirp.c | 23 ++++++++++++----- qemu-options.hx | 4 +- slirp/ip_icmp.c | 8 ++++-- slirp/ip_input.c | 29 +++-------------------- slirp/ip_output.c | 4 +- slirp/mbuf.h | 3 -- slirp/misc.c | 3 +- slirp/tcp_input.c | 10 ++++---- slirp/tcp_subr.c | 2 +- slirp/udp.c | 23 ++++++++++-------- 19 files changed, 102 insertions(+), 96 deletions(-)
PATCH 1/9 slirp: Fix restricted mode
This aligns the code to what the documentation claims: Allow everything but requests that would have to be routed outside of the virtual LAN.
So we need to drop the unneeded IP-level filter, allow TFTP requests, and add the missing protocol-level filter to ICMP.
--- slirp/ip_icmp.c | 2 ++ slirp/ip_input.c | 21 --------------------- slirp/udp.c | 8 ++++---- 3 files changed, 6 insertions(+), 25 deletions(-)
slirp/ip_icmp.c +2 -0
@@ -101,6 +101,8 @@ icmp_input(struct mbuf *m, int hlen) ip->ip_len += hlen;/* since ip_input subtracts this */ if (ip->ip_dst.s_addr == slirp->vhost_addr.s_addr) {icmp_reflect (m); + }else if (slirp->restricted) { + goto freeit; }else {struct socket *so;struct sockaddr_in addr;
slirp/ip_input.c +0 -21
@@ -118,27 +118,6 @@ ip_input(struct mbuf *m) goto bad; } -if (slirp->restricted) { -if ((ip->ip_dst.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -if (ip->ip_dst.s_addr ==0xffffffff && ip->ip_p !=IPPROTO_UDP ) - goto bad; - }else { -uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; -struct ex_list *ex_ptr; - -if ((ip->ip_dst.s_addr & inv_mask) == inv_mask) { - goto bad; - } -for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next) -if (ex_ptr->ex_addr.s_addr == ip->ip_dst.s_addr) -break ; - -if (!ex_ptr) - goto bad; - } - } -/* Should drop packet if mbuf too long? hmmm... */ if (m->m_len > ip->ip_len)m_adj (m, ip->ip_len - m->m_len);
slirp/udp.c +4 -4
@@ -125,10 +125,6 @@ udp_input(register struct mbuf *m, int iphlen) goto bad; } -if (slirp->restricted) { - goto bad; - } -/* * handle TFTP */
@@ -137,6 +133,10 @@ udp_input(register struct mbuf *m, int iphlen) goto bad; } +if (slirp->restricted) { + goto bad; + } +/* * Locate pcb for datagram. */
Signature · 1 line
1.7.1
PATCH 2/9 slirp: Canonicalize restrict syntax
In reply to Jan KiszkaAll other boolean arguments accept on|off - except for slirp's restrict. Fix that while still accepting the formerly allowed yes|y|no|n, but reject everything else. This avoids accidentally allowing external connections because syntax errors were so far interpreted as 'restrict=no'.
--- net/slirp.c | 21 +++++++++++++++------ qemu-options.hx | 4 ++-- 2 files changed, 17 insertions(+), 8 deletions(-)
net/slirp.c +15 -6
@@ -240,7 +240,8 @@ static int net_slirp_init(VLANState *vlan, const char *model, nc =qemu_new_net_client (&net_slirp_info, vlan,NULL , model, name);snprintf (nc->info_str,sizeof (nc->info_str), -"net=%s, restricted=%c" ,inet_ntoa (net), restricted ?'y' :'n' ); +"net=%s,restrict=%s" ,inet_ntoa (net), + restricted ?"on" :"off" ); s =DO_UPCAST (SlirpState, nc, nc);
@@ -689,6 +690,7 @@ int net_init_slirp(QemuOpts *opts,const char *bootfile;const char *smb_export;const char *vsmbsrv; +const char *restrict_opt;char *vnet =NULL ;int restricted =0 ;int ret;
@@ -702,6 +704,18 @@ int net_init_slirp(QemuOpts *opts, smb_export =qemu_opt_get (opts,"smb" ); vsmbsrv =qemu_opt_get (opts,"smbserver" ); + restrict_opt =qemu_opt_get (opts,"restrict" ); +if (restrict_opt) { +if (!strcmp (restrict_opt,"on" ) || + !strcmp (restrict_opt,"yes" ) || !strcmp (restrict_opt,"y" )) { + restricted =1 ; + }else if (strcmp (restrict_opt,"off" ) && +strcmp (restrict_opt,"no" ) &&strcmp (restrict_opt,"n" )) { +error_report ("invalid option: 'restrict=%s'" , restrict_opt); +return -1 ; + } + } +if (qemu_opt_get (opts,"ip" )) {const char *ip =qemu_opt_get (opts,"ip" );int l =strlen (ip) +strlen ("/24" ) +1 ;
@@ -720,11 +734,6 @@ int net_init_slirp(QemuOpts *opts, vnet =qemu_strdup (qemu_opt_get (opts,"net" )); } -if (qemu_opt_get (opts,"restrict" ) && -qemu_opt_get (opts,"restrict" )[0 ] =='y' ) { - restricted =1 ; - } -qemu_opt_foreach (opts, net_init_slirp_configs,NULL ,0 ); ret = net_slirp_init(vlan, "user", name, restricted, vnet, vhost,
qemu-options.hx +2 -2
@@ -1067,7 +1067,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, "-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n" " create a new Network Interface Card and connect it to VLAN 'n'\n" #ifdef CONFIG_SLIRP - "-net user[,vlan=n][,name=str][,net=addr[/mask]][,host=addr][,restrict=y|n]\n" + "-net user[,vlan=n][,name=str][,net=addr[/mask]][,host=addr][,restrict=on|off]\n" " [,hostname=host][,dhcpstart=addr][,dns=addr][,tftp=dir][,bootfile=f]\n" " [,hostfwd=rule][,guestfwd=rule]" #ifndef _WIN32
@@ -1160,7 +1160,7 @@ either in the form a.b.c.d or as number of valid top-most bits. Default is Specify the guest-visible address of the host. Default is the 2nd IP in the guest network, i.e. x.x.x.2. -@item restrict=y|yes|n|no +@item restrict=on|off If this options is enabled, the guest will be isolated, i.e. it will not be able to contact the host and no guest IP packets will be routed over the host to the outside. This option does not affect explicitly set forwarding rule.
Signature · 1 line
1.7.1
PATCH 3/9 slirp: Strictly associate DHCP/BOOTP and TFTP with virtual host
In reply to Jan KiszkaInstead of accepting every DHCP/BOOTP and TFTP packet, only invoke the built-in servers if the target is the virtual host.
--- slirp/udp.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)
slirp/udp.c +8 -5
@@ -120,15 +120,18 @@ udp_input(register struct mbuf *m, int iphlen)/* * handle DHCP/BOOTP */ -if (ntohs (uh->uh_dport) ==BOOTP_SERVER ) { -bootp_input (m); - goto bad; - } +if (ntohs (uh->uh_dport) ==BOOTP_SERVER && + (ip->ip_dst.s_addr == slirp->vhost_addr.s_addr || + ip->ip_dst.s_addr ==0xffffffff )) { +bootp_input (m); + goto bad; + }/* * handle TFTP */ -if (ntohs (uh->uh_dport) ==TFTP_SERVER ) { +if (ntohs (uh->uh_dport) ==TFTP_SERVER && + ip->ip_dst.s_addr == slirp->vhost_addr.s_addr) {tftp_input (m); goto bad; }
Signature · 1 line
1.7.1
PATCH 4/9 slirp: Replace m_freem with m_free
In reply to Jan KiszkaRemove this pointless wrapping.
--- slirp/ip_icmp.c | 6 +++--- slirp/ip_input.c | 8 ++++---- slirp/ip_output.c | 4 ++-- slirp/mbuf.h | 3 --- slirp/tcp_input.c | 10 +++++----- slirp/tcp_subr.c | 2 +- slirp/udp.c | 2 +- 7 files changed, 16 insertions(+), 19 deletions(-)
slirp/ip_icmp.c +3 -3
@@ -81,7 +81,7 @@ icmp_input(struct mbuf *m, int hlen)*/ if (icmplen <ICMP_MINLEN ) {/* min 8 bytes payload */ freeit: -m_freem (m); +m_free (m); goto end_error; }
@@ -155,11 +155,11 @@ icmp_input(struct mbuf *m, int hlen)case ICMP_TSTAMP :case ICMP_MASKREQ :case ICMP_REDIRECT : -m_freem (m); +m_free (m);break ;default : -m_freem (m); +m_free (m); }/* swith */ end_error:
slirp/ip_input.c +4 -4
@@ -204,7 +204,7 @@ ip_input(struct mbuf *m) }return ; bad: -m_freem (m); +m_free (m);return ; }
@@ -297,7 +297,7 @@ ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)break ; } q = q->ipf_next; -m_freem (dtom (slirp, q->ipf_prev)); +m_free (dtom (slirp, q->ipf_prev));ip_deq (q->ipf_prev); }
@@ -363,7 +363,7 @@ insert:return ip; dropfrag: -m_freem (m); +m_free (m);return NULL ; }
@@ -379,7 +379,7 @@ ip_freef(Slirp *slirp, struct ipq *fp)for (q = fp->frag_link.next; q != (struct ipasfrag *)&fp->frag_link; q = p) { p = q->ipf_next;ip_deq (q); -m_freem (dtom (slirp, q)); +m_free (dtom (slirp, q)); }remque (&fp->ip_link); (void )m_free (dtom (slirp, fp));
slirp/ip_output.c +2 -2
@@ -159,7 +159,7 @@ sendorfree:if (error ==0 )if_output (so, m);else -m_freem (m); +m_free (m); } }
@@ -167,6 +167,6 @@ done:return (error); bad: -m_freem (m0); +m_free (m0); goto done; }
slirp/mbuf.h +0 -3
@@ -33,9 +33,6 @@#ifndef _MBUF_H_#define _MBUF_H_ -#define m_freem m_free - -#define MINCSIZE 4096/* Amount to increase mbuf if too small */ /*
slirp/tcp_input.c +5 -5
@@ -136,7 +136,7 @@ tcp_reass(register struct tcpcb *tp, register struct tcpiphdr *ti, i = q->ti_seq + q->ti_len - ti->ti_seq;if (i >0 ) {if (i >= ti->ti_len) { -m_freem (m); +m_free (m);/* * Try to present any queued data * at the left window edge to the user.
@@ -170,7 +170,7 @@ tcp_reass(register struct tcpcb *tp, register struct tcpiphdr *ti, q =tcpiphdr_next (q); m =tcpiphdr_prev (q)->ti_mbuf;remque (tcpiphdr2qlink (tcpiphdr_prev (q))); -m_freem (m); +m_free (m); }/*
@@ -197,7 +197,7 @@ present: m = ti->ti_mbuf; ti =tcpiphdr_next (ti);if (so->so_state &SS_FCANTSENDMORE ) -m_freem (m); +m_free (m);else {if (so->so_emu) {if (tcp_emu (so,m))sbappend (so, m);
@@ -451,7 +451,7 @@ findso: acked = ti->ti_ack - tp->snd_una;sbdrop (&so->so_snd, acked); tp->snd_una = ti->ti_ack; -m_freem (m); +m_free (m);/* * If all outstanding data are acked, stop
@@ -1260,7 +1260,7 @@ dropafterack:*/ if (tiflags &TH_RST ) goto drop; -m_freem (m); +m_free (m); tp->t_flags |=TF_ACKNOW ; (void )tcp_output (tp);return ;
slirp/tcp_subr.c +1 -1
@@ -250,7 +250,7 @@ tcp_close(struct tcpcb *tp) t =tcpiphdr_next (t); m =tcpiphdr_prev (t)->ti_mbuf;remque (tcpiphdr2qlink (tcpiphdr_prev (t))); -m_freem (m); +m_free (m); }free (tp); so->so_tcpcb =NULL ;
slirp/udp.c +1 -1
@@ -222,7 +222,7 @@ udp_input(register struct mbuf *m, int iphlen)return ; bad: -m_freem (m); +m_free (m);return ; }
Signature · 1 line
1.7.1
PATCH 5/9 slirp: Put forked exec into separate process group
In reply to Jan KiszkaRecent smb daemons tend to terminate themselves via a process group SIGTERM. If the daemon is still in qemu's group by that time, qemu will die as well. Avoid this by always pushing fork_exec processes into a group of their own, not just (unused) type 2 execs.
--- slirp/misc.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
slirp/misc.c +2 -1
@@ -153,11 +153,12 @@ fork_exec(struct socket *so, const char *ex, int do_pty)return 0 ;case 0 : +setsid (); +/* Set the DISPLAY */ if (do_pty ==2 ) { (void )close (master);#ifdef TIOCSCTTY /* XXXXX */ - (void )setsid ();ioctl (s,TIOCSCTTY , (char *)NULL );#endif }else {
Signature · 1 line
1.7.1
PATCH 6/9 net: Improve layout of 'info network'
In reply to Jan KiszkaImprove the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:
orphan: net=10.0.2.0, restricted=n
virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network2: fd=5
e1000.0: model=e1000,macaddr=52:54:00:12:34:57
\ network1: net=10.0.2.0, restricted=n
rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58
ie. peers are grouped, orphans are listed as before.
--- net.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
net.c +9 -5
@@ -1224,7 +1224,8 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)void do_info_network (Monitor *mon) {VLANState *vlan; -VLANClientState *vc; +VLANClientState *vc, *peer; +net_client_type type; QTAILQ_FOREACH(vlan, &vlans, next) {monitor_printf (mon,"VLAN %d devices:\n" , vlan->id);
@@ -1235,11 +1236,14 @@ void do_info_network(Monitor *mon) }monitor_printf (mon,"Devices not on any VLAN:\n" ); QTAILQ_FOREACH(vc, &non_vlan_clients, next) { -monitor_printf (mon," %s: %s" , vc->name, vc->info_str); -if (vc->peer) { -monitor_printf (mon," peer=%s" , vc->peer->name); + peer = vc->peer; + type = vc->info->type; +if (!peer || type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); + } +if (peer && type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); } -monitor_printf (mon,"\n" ); } }
Signature · 1 line
1.7.1
Re: [Qemu-devel] [PATCH 6/9] net: Improve layout of 'info network'
Quoted 12 lines · Jan Kiszka
Improve the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:orphan: net=10.0.2.0, restricted=n virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 \ network2: fd=5 e1000.0: model=e1000,macaddr=52:54:00:12:34:57 \ network1: net=10.0.2.0, restricted=n rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58
Quoted 4 lines
ie. peers are grouped, orphans are listed as before.
CC Markus Armbruster<[email protected]>Signed-off-by Jan Kiszka<[email protected]>
There isn't a query-network yet in QMP so libvirt is probably still using the HMP version.
Can someone on the libvirt side Ack/Nack about whether this patch will break libvirt?
Regards,
Anthony Liguori
Quoted 37 lines
--- net.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)net.c +9
-5@@ -1224,7 +1224,8 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)void do_info_network (Monitor *mon) {VLANState *vlan; -VLANClientState *vc; +VLANClientState *vc, *peer; +net_client_type type; QTAILQ_FOREACH(vlan,&vlans, next) {monitor_printf (mon,"VLAN %d devices:\n" , vlan->id);@@ -1235,11 +1236,14 @@ void do_info_network(Monitor *mon) }monitor_printf (mon,"Devices not on any VLAN:\n" ); QTAILQ_FOREACH(vc,&non_vlan_clients, next) { -monitor_printf (mon," %s: %s" , vc->name, vc->info_str); -if (vc->peer) { -monitor_printf (mon," peer=%s" , vc->peer->name); + peer = vc->peer; + type = vc->info->type; +if (!peer || type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); + } +if (peer&& type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); } -monitor_printf (mon,"\n" ); } }
Re: [Qemu-devel] [PATCH 6/9] net: Improve layout of 'info network'
Quoted 15 lines · Anthony Liguori
Quoted 14 lines · Jan Kiszka
Improve the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:orphan: net=10.0.2.0, restricted=n virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 \ network2: fd=5 e1000.0: model=e1000,macaddr=52:54:00:12:34:57 \ network1: net=10.0.2.0, restricted=n rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58ie. peers are grouped, orphans are listed as before.
Quoted 9 lines
Quoted 2 lines
CC Markus Armbruster<[email protected]>Signed-off-by Jan Kiszka<[email protected]>There isn't a query-network yet in QMP so libvirt is probably still using the HMP version.
Can someone on the libvirt side Ack/Nack about whether this patch will break libvirt?
libvirt doesn't use 'info network'
- Cole
Re: [Qemu-devel] [PATCH 6/9] net: Improve layout of 'info network'
In reply to Jan KiszkaQuoted 36 lines · Jan Kiszka
Improve the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:
orphan: net=10.0.2.0, restricted=n
virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network2: fd=5
e1000.0: model=e1000,macaddr=52:54:00:12:34:57
\ network1: net=10.0.2.0, restricted=n
rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58ie. peers are grouped, orphans are listed as before.
CC Markus Armbruster <[email protected]>Signed-off-by Jan Kiszka <[email protected]>--- net.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)net.c +2
-1@@ -1224,7 +1224,8 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)void do_info_network (Monitor *mon) {VLANState *vlan; -VLANClientState *vc; +VLANClientState *vc, *peer; +net_client_type type; QTAILQ_FOREACH(vlan, &vlans, next) {monitor_printf (mon,"VLAN %d devices:\n" , vlan->id);@@ -1235,11 +1236,14 @@ void do_info_network(Monitor *mon) }
net.c void do_info_network(Monitor *mon) · line 1243
Quoted 10 lines
net.c +5
-3monitor_printf (mon,"Devices not on any VLAN:\n" ); QTAILQ_FOREACH(vc, &non_vlan_clients, next) { -monitor_printf (mon," %s: %s" , vc->name, vc->info_str); -if (vc->peer) { -monitor_printf (mon," peer=%s" , vc->peer->name); + peer = vc->peer; + type = vc->info->type; +if (!peer || type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); + }
I still think we could use a comment here, like
} /* else it's a netdev connected to a NIC, printed with the NIC */
See http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg01603.html
Quoted 6 lines
net.c +2
-1+if (peer && type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); } -monitor_printf (mon,"\n" ); } }
PATCH v2 6/9 net: Improve layout of 'info network'
Improve the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:
orphan: net=10.0.2.0, restricted=n
virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network2: fd=5
e1000.0: model=e1000,macaddr=52:54:00:12:34:57
\ network1: net=10.0.2.0, restricted=n
rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58
ie. peers are grouped, orphans are listed as before.
---
v2: added comment as suggested by Markus
net.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
net.c +9 -5
@@ -1224,7 +1224,8 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)void do_info_network (Monitor *mon) {VLANState *vlan; -VLANClientState *vc; +VLANClientState *vc, *peer; +net_client_type type; QTAILQ_FOREACH(vlan, &vlans, next) {monitor_printf (mon,"VLAN %d devices:\n" , vlan->id);
@@ -1235,11 +1236,14 @@ void do_info_network(Monitor *mon) }monitor_printf (mon,"Devices not on any VLAN:\n" ); QTAILQ_FOREACH(vc, &non_vlan_clients, next) { -monitor_printf (mon," %s: %s" , vc->name, vc->info_str); -if (vc->peer) { -monitor_printf (mon," peer=%s" , vc->peer->name); + peer = vc->peer; + type = vc->info->type; +if (!peer || type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); + }/* else it's a netdev connected to a NIC, printed with the NIC */ +if (peer && type ==NET_CLIENT_TYPE_NIC ) { +monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); } -monitor_printf (mon,"\n" ); } }
Signature · 1 line
1.7.1
Re: [Qemu-devel] [PATCH v2 6/9] net: Improve layout of 'info network'
Quoted 12 lines · Jan Kiszka
Improve the layout when listing non-vlan clients via 'info network'. The result looks like this:
(qemu) info network
Devices not on any VLAN:
orphan: net=10.0.2.0, restricted=n
virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network2: fd=5
e1000.0: model=e1000,macaddr=52:54:00:12:34:57
\ network1: net=10.0.2.0, restricted=n
rtl8139.0: model=rtl8139,macaddr=52:54:00:12:34:58
Quoted 7 lines
ie. peers are grouped, orphans are listed as before.
CC Markus Armbruster <[email protected]>Signed-off-by Jan Kiszka <[email protected]>---
v2: added comment as suggested by Markus
Looks good (rebased 8/9 too), thanks!
PATCH 7/9 net: Refactor net_client_types
In reply to Jan KiszkaPosition entries of net_client_types according to the corresponding values of NET_CLIENT_TYPE_*. The array size is now defined by NET_CLIENT_TYPE_MAX. This will allow to obtain entries based on type value in later patches.
At this chance rename NET_CLIENT_TYPE_SLIRP to NET_CLIENT_TYPE_USER for the sake of consistency.
--- net.c | 30 ++++++++++++++++++------------ net.h | 6 ++++-- net/slirp.c | 2 +- 3 files changed, 23 insertions(+), 15 deletions(-)
net.c +18 -12
@@ -830,14 +830,15 @@ static const struct {const char *type;net_client_init_func init;QemuOptDesc desc[NET_MAX_DESC ]; -} net_client_types[] = { - { +} net_client_types[NET_CLIENT_TYPE_MAX] = { + [NET_CLIENT_TYPE_NONE] = { .type = "none", .desc = { NET_COMMON_PARAMS_DESC, {/* end of list */ } }, - }, { + }, + [NET_CLIENT_TYPE_NIC] = { .type = "nic", .init = net_init_nic, .desc = {
@@ -866,8 +867,9 @@ static const struct { }, {/* end of list */ } }, + }, #ifdef CONFIG_SLIRP - }, { + [NET_CLIENT_TYPE_USER] = { .type = "user", .init = net_init_slirp, .desc = {
@@ -927,8 +929,9 @@ static const struct { }, {/* end of list */ } }, + }, #endif - }, { + [NET_CLIENT_TYPE_TAP] = { .type = "tap", .init = net_init_tap, .desc = {
@@ -975,7 +978,8 @@ static const struct {#endif /* _WIN32 */ {/* end of list */ } }, - }, { + }, + [NET_CLIENT_TYPE_SOCKET] = { .type = "socket", .init = net_init_socket, .desc = {
@@ -1003,8 +1007,9 @@ static const struct { }, {/* end of list */ } }, + }, #ifdef CONFIG_VDE - }, { + [NET_CLIENT_TYPE_VDE] = { .type = "vde", .init = net_init_vde, .desc = {
@@ -1028,8 +1033,9 @@ static const struct { }, {/* end of list */ } }, + }, #endif - }, { + [NET_CLIENT_TYPE_DUMP] = { .type = "dump", .init = net_init_dump, .desc = {
@@ -1046,7 +1052,6 @@ static const struct { {/* end of list */ } }, }, - {/* end of list */ } };int net_client_init (Monitor *mon,QemuOpts *opts,int is_netdev)
@@ -1094,8 +1099,9 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev) name =qemu_opt_get (opts,"name" ); } -for (i =0 ; net_client_types[i].type !=NULL ; i++) { -if (!strcmp (net_client_types[i].type, type)) { +for (i =0 ; i <NET_CLIENT_TYPE_MAX ; i++) { +if (net_client_types[i].type !=NULL && + !strcmp (net_client_types[i].type, type)) {VLANState *vlan =NULL ;int ret;
@@ -1334,7 +1340,7 @@ void net_check_clients(void) seen_nics++; has_nic =1 ;break ; -case NET_CLIENT_TYPE_SLIRP : +case NET_CLIENT_TYPE_USER :case NET_CLIENT_TYPE_TAP :case NET_CLIENT_TYPE_SOCKET :case NET_CLIENT_TYPE_VDE :
net.h +4 -2
@@ -31,11 +31,13 @@ typedef struct NICConf {typedef enum {NET_CLIENT_TYPE_NONE ,NET_CLIENT_TYPE_NIC , -NET_CLIENT_TYPE_SLIRP , +NET_CLIENT_TYPE_USER ,NET_CLIENT_TYPE_TAP ,NET_CLIENT_TYPE_SOCKET ,NET_CLIENT_TYPE_VDE , -NET_CLIENT_TYPE_DUMP +NET_CLIENT_TYPE_DUMP , + +NET_CLIENT_TYPE_MAX }net_client_type ;typedef void (NetPoll )(VLANClientState *,bool enable);
net/slirp.c +1 -1
@@ -128,7 +128,7 @@ static void net_slirp_cleanup(VLANClientState *nc) }static NetClientInfo net_slirp_info = { - .type =NET_CLIENT_TYPE_SLIRP , + .type =NET_CLIENT_TYPE_USER , .size =sizeof (SlirpState), .receive = net_slirp_receive, .cleanup = net_slirp_cleanup,
Signature · 1 line
1.7.1
PATCH 8/9 net: Dump client type 'info network'
In reply to Jan KiszkaInclude the client type name into the output of 'info network'. The result looks like this:
(qemu) info network
VLAN 0 devices:
rtl8139.0: type=nic,model=rtl8139,macaddr=52:54:00:12:34:57
Devices not on any VLAN:
virtio-net-pci.0: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network1: type=tap,fd=5
--- net.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-)
net.c +12 -3
@@ -1227,6 +1227,12 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)return 0 ; } +static void print_net_client (Monitor *mon,VLANClientState *vc) +{ +monitor_printf (mon,"%s: type=%s,%s\n" , vc->name, + net_client_types[vc->info->type].type, vc->info_str); +} +void do_info_network (Monitor *mon) {VLANState *vlan;
@@ -1237,7 +1243,8 @@ void do_info_network(Monitor *mon)monitor_printf (mon,"VLAN %d devices:\n" , vlan->id); QTAILQ_FOREACH(vc, &vlan->clients, next) { -monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); +monitor_printf (mon," " ); +print_net_client (mon, vc); } }monitor_printf (mon,"Devices not on any VLAN:\n" );
@@ -1245,10 +1252,12 @@ void do_info_network(Monitor *mon) peer = vc->peer; type = vc->info->type;if (!peer || type ==NET_CLIENT_TYPE_NIC ) { -monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); +monitor_printf (mon," " ); +print_net_client (mon, vc); }if (peer && type ==NET_CLIENT_TYPE_NIC ) { -monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); +monitor_printf (mon," \\ " ); +print_net_client (mon, peer); } } }
Signature · 1 line
1.7.1
PATCH v2 8/9 net: Dump client type 'info network'
Include the client type name into the output of 'info network'. The result looks like this:
(qemu) info network
VLAN 0 devices:
rtl8139.0: type=nic,model=rtl8139,macaddr=52:54:00:12:34:57
Devices not on any VLAN:
virtio-net-pci.0: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ network1: type=tap,fd=5
---
v2: Rebased over changes to 6/9
net.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-)
net.c +12 -3
@@ -1227,6 +1227,12 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)return 0 ; } +static void print_net_client (Monitor *mon,VLANClientState *vc) +{ +monitor_printf (mon,"%s: type=%s,%s\n" , vc->name, + net_client_types[vc->info->type].type, vc->info_str); +} +void do_info_network (Monitor *mon) {VLANState *vlan;
@@ -1237,7 +1243,8 @@ void do_info_network(Monitor *mon)monitor_printf (mon,"VLAN %d devices:\n" , vlan->id); QTAILQ_FOREACH(vc, &vlan->clients, next) { -monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); +monitor_printf (mon," " ); +print_net_client (mon, vc); } }monitor_printf (mon,"Devices not on any VLAN:\n" );
@@ -1245,10 +1252,12 @@ void do_info_network(Monitor *mon) peer = vc->peer; type = vc->info->type;if (!peer || type ==NET_CLIENT_TYPE_NIC ) { -monitor_printf (mon," %s: %s\n" , vc->name, vc->info_str); +monitor_printf (mon," " ); +print_net_client (mon, vc); }/* else it's a netdev connected to a NIC, printed with the NIC */ if (peer && type ==NET_CLIENT_TYPE_NIC ) { -monitor_printf (mon," \\ %s: %s\n" , peer->name, peer->info_str); +monitor_printf (mon," \\ " ); +print_net_client (mon, peer); } } }
PATCH 9/9 net: Consistently use qemu_macaddr_default_if_unset
In reply to Jan KiszkaDrop the open-coded MAC assignment from net_init_nic and replace it with standard qemu_macaddr_default_if_unset which is also used by qdev. That avoid creating colliding MACs when instantiating NICs via different mechanisms.
This change requires to store the MAC as MACAddr in NICInfo, and the remaining nd_table users need to be updated.
Based on suggestion by Peter Maydell.
--- hw/dp8393x.c | 2 +- hw/etraxfs_eth.c | 2 +- hw/mcf_fec.c | 2 +- hw/mipsnet.c | 2 +- hw/qdev.c | 2 +- hw/stellaris.c | 2 +- hw/xen_devconfig.c | 4 ++-- net.c | 10 ++-------- net.h | 2 +- 9 files changed, 11 insertions(+), 17 deletions(-)
hw/dp8393x.c +1 -1
@@ -898,7 +898,7 @@ void dp83932_init(NICInfo *nd, target_phys_addr_t base, int it_shift, s->watchdog =qemu_new_timer_ns (vm_clock, dp8393x_watchdog, s); s->regs[SONIC_SR ] =0x0004 ;/* only revision recognized by Linux */ -memcpy (s->conf.macaddr.a, nd->macaddr,sizeof (s->conf.macaddr)); + s->conf.macaddr = nd->macaddr; s->conf.vlan = nd->vlan; s->conf.peer = nd->netdev;
hw/etraxfs_eth.c +1 -1
@@ -602,7 +602,7 @@ void *etraxfs_eth_init(NICInfo *nd, target_phys_addr_t base, int phyaddr) DEVICE_NATIVE_ENDIAN);cpu_register_physical_memory (base,0x5c , eth->ethregs); -memcpy (eth->conf.macaddr.a, nd->macaddr,sizeof (nd->macaddr)); + eth->conf.macaddr = nd->macaddr; eth->conf.vlan = nd->vlan; eth->conf.peer = nd->netdev;
hw/mcf_fec.c +1 -1
@@ -471,7 +471,7 @@ void mcf_fec_init(NICInfo *nd, target_phys_addr_t base, qemu_irq *irq) DEVICE_NATIVE_ENDIAN);cpu_register_physical_memory (base,0x400 , s->mmio_index); -memcpy (s->conf.macaddr.a, nd->macaddr,sizeof (nd->macaddr)); + s->conf.macaddr = nd->macaddr; s->conf.vlan = nd->vlan; s->conf.peer = nd->netdev;
hw/mipsnet.c +1 -1
@@ -258,7 +258,7 @@ void mipsnet_init (int base, qemu_irq irq, NICInfo *nd) s->irq = irq;if (nd) { -memcpy (s->conf.macaddr.a, nd->macaddr,sizeof (nd->macaddr)); + s->conf.macaddr = nd->macaddr; s->conf.vlan = nd->vlan; s->conf.peer = nd->netdev;
hw/qdev.c +1 -1
@@ -459,7 +459,7 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)void qdev_set_nic_properties (DeviceState *dev,NICInfo *nd) { -qdev_prop_set_macaddr (dev,"mac" , nd->macaddr); +qdev_prop_set_macaddr (dev,"mac" , nd->macaddr.a);if (nd->vlan)qdev_prop_set_vlan (dev,"vlan" , nd->vlan);if (nd->netdev)
hw/stellaris.c +1 -1
@@ -1230,7 +1230,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, } } -stellaris_sys_init (0x400fe000 , pic[28 ], board, nd_table[0 ].macaddr); +stellaris_sys_init (0x400fe000 , pic[28 ], board, nd_table[0 ].macaddr.a);for (i =0 ; i <7 ; i++) {if (board->dc4 & (1 << i)) {
hw/xen_devconfig.c +2 -2
@@ -126,8 +126,8 @@ int xen_config_dev_nic(NICInfo *nic)char mac[20 ];snprintf (mac,sizeof (mac),"%02x:%02x:%02x:%02x:%02x:%02x" , - nic->macaddr[0 ], nic->macaddr[1 ], nic->macaddr[2 ], - nic->macaddr[3 ], nic->macaddr[4 ], nic->macaddr[5 ]); + nic->macaddr.a[0 ], nic->macaddr.a[1 ], nic->macaddr.a[2 ], + nic->macaddr.a[3 ], nic->macaddr.a[4 ], nic->macaddr.a[5 ]);xen_be_printf (NULL ,1 ,"config nic %d: mac=\"%s\"\n" , nic->vlan->id, mac);xen_config_dev_dirs ("vif" ,"qnic" , nic->vlan->id, fe, be,sizeof (fe));
net.c +2 -8
@@ -776,18 +776,12 @@ static int net_init_nic(QemuOpts *opts, nd->devaddr =qemu_strdup (qemu_opt_get (opts,"addr" )); } - nd->macaddr[0 ] =0x52 ; - nd->macaddr[1 ] =0x54 ; - nd->macaddr[2 ] =0x00 ; - nd->macaddr[3 ] =0x12 ; - nd->macaddr[4 ] =0x34 ; - nd->macaddr[5 ] =0x56 + idx; -if (qemu_opt_get (opts,"macaddr" ) && -net_parse_macaddr (nd->macaddr,qemu_opt_get (opts,"macaddr" )) <0 ) { +net_parse_macaddr (nd->macaddr.a,qemu_opt_get (opts,"macaddr" )) <0 ) {error_report ("invalid syntax for ethernet address" );return -1 ; } +qemu_macaddr_default_if_unset (&nd->macaddr); nd->nvectors =qemu_opt_get_number (opts,"vectors" ,DEV_NVECTORS_UNSPECIFIED );
net.h +1 -1
@@ -129,7 +129,7 @@ int do_set_link(Monitor *mon, const QDict *qdict, QObject **ret_data);#define MAX_NICS 8struct NICInfo { -uint8_t macaddr[6 ]; +MACAddr macaddr;char *model;char *name;char *devaddr;
Signature · 1 line
1.7.1
6-9/9 look good. I didn't review the slirp stuff.