slirp: Adding IPv6 support to Qemu -net user mode
v5 cover slirp: Adding IPv6 support to Qemu -net user mode
Hello,
This is another respin of IPv6 in Qemu -net user mode.
These patches add ICMPv6, NDP, and make UDP and TCP compatible with IPv6. We have made some refactoring to make current code compatible with IPv6.
Patches 1 to 8 are refactoring of existing code and do not change the behavior, 9 adds a helper, and 10 to 18 add ipv6 support. Patches 3 and 16 are only reindents of the resulting code, without code change.
Difference with version 4 is:
- rebase on qemu 2.5
Here is a summary of the patches:
Refactoring patches:
[PATCH 01/18] slirp: goto bad in udp_input if sosendto fails [PATCH 02/18] slirp: Generalizing and neutralizing code before adding [PATCH 03/18] slirp: Reindent after refactoring
[PATCH 04/18] slirp: Make Socket structure IPv6 compatible [PATCH 05/18] slirp: Factorizing address translation [PATCH 06/18] slirp: Factorizing and cleaning solookup() [PATCH 07/18] slirp: Make udp_attach IPv6 compatible [PATCH 08/18] slirp: Adding family argument to tcp_fconnect()
Adding helper:
[PATCH 09/18] qemu/timer.h : Adding function to second scale
Adding v6 support:
[PATCH 10/18] slirp: Adding IPv6, ICMPv6 Echo and NDP [PATCH 11/18] slirp: Adding ICMPv6 error sending
[PATCH 12/18] slirp: Adding IPv6 UDP support
[PATCH 13/18] slirp: Factorizing tcpiphdr structure with an union [PATCH 14/18] slirp: Generalizing and neutralizing various TCP [PATCH 15/18] slirp: Reindent after refactoring
[PATCH 16/18] slirp: Handle IPv6 in TCP functions
[PATCH 17/18] slirp: Adding IPv6 address for DNS relay [PATCH 18/18] qapi-schema, qemu-options & slirp: Adding Qemu options
PATCH 1/18 slirp: goto bad in udp_input if sosendto fails
From: Guillaume Subiron <[email protected]>
Before this patch, if sosendto fails, udp_input is executed as if the packet was sent, recording the packet for icmp errors, which does not makes sense since the packet was not actually sent, errors would be related to a previous packet.
This patch adds a goto bad to cut the execution of this function.
--- slirp/debug.h | 2 +- slirp/udp.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
slirp/debug.h +1 -1
@@ -5,7 +5,7 @@* terms and conditions of the copyright. */ -//#define DEBUG 1 +#define DEBUG 1#ifdef DEBUG
slirp/udp.c +1 -0
@@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen) *ip=save_ip;DEBUG_MISC ((dfd,"udp tx errno = %d-%s\n" ,errno,strerror (errno)));icmp_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno)); + goto bad; }m_free (so->so_m);/* used for ICMP if error on sorecvfrom */
Signature · 1 line
2.6.2
PATCH 2/18 slirp: Generalizing and neutralizing code before adding IPv6 stuff
From: Guillaume Subiron <[email protected]>
Basically, this patch replaces "arp" by "resolution" every time "arp" means "mac resolution" and not specifically ARP.
Some indentation problems are solved in functions that will be modified in the next patches (ip_input…).
In if_encap, a switch is added to prepare for the IPv6 case. Some code is factorized.
--- slirp/mbuf.c | 2 +- slirp/mbuf.h | 2 +- slirp/slirp.c | 24 ++++++++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-)
slirp/mbuf.c +1 -1
@@ -91,7 +91,7 @@ m_get(Slirp *slirp) m->m_len =0 ; m->m_nextpkt =NULL ; m->m_prevpkt =NULL ; - m->arp_requested = false; + m->resolution_requested = false; m->expiration_date = (uint64_t )-1 ; end_error:DEBUG_ARG ("m = %p" , m);
slirp/mbuf.h +1 -1
@@ -79,7 +79,7 @@ struct mbuf {int m_len;/* Amount of data in this mbuf */ Slirp *slirp; -bool arp_requested; +bool resolution_requested;uint64_t expiration_date;/* start of dynamic buffer area, must be last element */ union {
slirp/slirp.c +20 -4
@@ -776,6 +776,8 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)return 1 ; } +switch (iph->ip_v) { +case IPVERSION :if (iph->ip_dst.s_addr ==0 ) {/* 0.0.0.0 can not be a destination address, something went wrong, * avoid making it worse */
@@ -786,7 +788,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)struct ethhdr *reh = (struct ethhdr *)arp_req;struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); -if (!ifm->arp_requested) { +if (!ifm->resolution_requested) {/* If the client addr is not known, send an ARP request */ memset (reh->h_dest,0xff ,ETH_ALEN );memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 );
@@ -812,22 +814,36 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) rah->ar_tip = iph->ip_dst.s_addr; slirp->client_ipaddr = iph->ip_dst;slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); - ifm->arp_requested = true; + ifm->resolution_requested = true;/* Expire request and drop outgoing packet after 1 second */ ifm->expiration_date =qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; }return 0 ; }else { -memcpy (eh->h_dest, ethaddr,ETH_ALEN );memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 );/* XXX: not correct */ memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); eh->h_proto =htons (ETH_P_IP ); +break ; + } + +default : +/* Do not assert while we don't manage IP6VERSION */ +/* assert(0); */ +break ; + } + +memcpy (eh->h_dest, ethaddr,ETH_ALEN ); +DEBUG_ARGS ((dfd," src = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], + eh->h_source[3 ], eh->h_source[4 ], eh->h_source[5 ])); +DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], + eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ]));memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len);slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN );return 1 ; - } }/* Drop host forwarding rule, return 0 if found. */
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 50 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
Basically, this patch replaces "arp" by "resolution" every time "arp" means "mac resolution" and not specifically ARP.
Some indentation problems are solved in functions that will be modified in the next patches (ip_input…).
In
if_encap, a switch is added to prepare for the IPv6 case. Some code is factorized.Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/mbuf.c | 2 +- slirp/mbuf.h | 2 +- slirp/slirp.c | 24 ++++++++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-)slirp/mbuf.c +1
-1@@ -91,7 +91,7 @@ m_get(Slirp *slirp) m->m_len =0 ; m->m_nextpkt =NULL ; m->m_prevpkt =NULL ; - m->arp_requested = false; + m->resolution_requested = false; m->expiration_date = (uint64_t )-1 ; end_error:DEBUG_ARG ("m = %p" , m);slirp/mbuf.h +1
-1@@ -79,7 +79,7 @@ struct mbuf {int m_len;/* Amount of data in this mbuf */ Slirp *slirp; -bool arp_requested; +bool resolution_requested;uint64_t expiration_date;/* start of dynamic buffer area, must be last element */ union {slirp/slirp.c +0
-0
slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 783
Quoted 8 lines
slirp/slirp.c +2
-0@@ -776,6 +776,8 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)return 1 ; } +switch (iph->ip_v) { +case IPVERSION :if (iph->ip_dst.s_addr ==0 ) {/* 0.0.0.0 can not be a destination address, something went wrong, * avoid making it worse */
That indentation looks now broken - shouldn't the if-statement now be indented with four more spaces now?
Quoted 37 lines
slirp/slirp.c +13
-3@@ -786,7 +788,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)struct ethhdr *reh = (struct ethhdr *)arp_req;struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); -if (!ifm->arp_requested) { +if (!ifm->resolution_requested) {/* If the client addr is not known, send an ARP request */ memset (reh->h_dest,0xff ,ETH_ALEN );memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 );@@ -812,22 +814,36 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) rah->ar_tip = iph->ip_dst.s_addr; slirp->client_ipaddr = iph->ip_dst;slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); - ifm->arp_requested = true; + ifm->resolution_requested = true;/* Expire request and drop outgoing packet after 1 second */ ifm->expiration_date =qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; }return 0 ; }else { -memcpy (eh->h_dest, ethaddr,ETH_ALEN );memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 );/* XXX: not correct */ memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); eh->h_proto =htons (ETH_P_IP ); +break ; + } + +default : +/* Do not assert while we don't manage IP6VERSION */ +/* assert(0); */ +break ; + } + +memcpy (eh->h_dest, ethaddr,ETH_ALEN ); + DEBUG_ARGS((dfd, " src = %02x:%02x:%02x:%02x:%02x:%02x\n",
slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 847
Quoted 10 lines
slirp/slirp.c +5
-1+ eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], + eh->h_source[3], eh->h_source[4], eh->h_source[5])); +DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], + eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ]));memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len);slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN );return 1 ; - } }
The lines after the switch statement should also only be indented with 4 spaces, not with 8.
Also, I'd prefer if you could split this patch into two: One for renaming the "arp_requested" field, and one for adding the additional logic with the switch statement (since there are two different kind of changes).
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 105 lines · Thomas Huth
Quoted 50 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
Basically, this patch replaces "arp" by "resolution" every time "arp" means "mac resolution" and not specifically ARP.
Some indentation problems are solved in functions that will be modified in the next patches (ip_input…).
In
if_encap, a switch is added to prepare for the IPv6 case. Some code is factorized.Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/mbuf.c | 2 +- slirp/mbuf.h | 2 +- slirp/slirp.c | 24 ++++++++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-)slirp/mbuf.c +1
-1@@ -91,7 +91,7 @@ m_get(Slirp *slirp) m->m_len =0 ; m->m_nextpkt =NULL ; m->m_prevpkt =NULL ; - m->arp_requested = false; + m->resolution_requested = false; m->expiration_date = (uint64_t )-1 ; end_error:DEBUG_ARG ("m = %p" , m);slirp/mbuf.h +1
-1@@ -79,7 +79,7 @@ struct mbuf {int m_len;/* Amount of data in this mbuf */ Slirp *slirp; -bool arp_requested; +bool resolution_requested;uint64_t expiration_date;/* start of dynamic buffer area, must be last element */ union {slirp/slirp.c +0
-0slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 783
Quoted 8 lines
slirp/slirp.c +2
-0@@ -776,6 +776,8 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)return 1 ; } +switch (iph->ip_v) { +case IPVERSION :if (iph->ip_dst.s_addr ==0 ) {/* 0.0.0.0 can not be a destination address, something went wrong, * avoid making it worse */ That indentation looks now broken - shouldn't the if-statement now be indented with four more spaces now?
Quoted 42 lines
slirp/slirp.c +18
-3@@ -786,7 +788,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)struct ethhdr *reh = (struct ethhdr *)arp_req;struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); -if (!ifm->arp_requested) { +if (!ifm->resolution_requested) {/* If the client addr is not known, send an ARP request */ memset (reh->h_dest,0xff ,ETH_ALEN );memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 );@@ -812,22 +814,36 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) rah->ar_tip = iph->ip_dst.s_addr; slirp->client_ipaddr = iph->ip_dst;slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); - ifm->arp_requested = true; + ifm->resolution_requested = true;/* Expire request and drop outgoing packet after 1 second */ ifm->expiration_date =qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; }return 0 ; }else { -memcpy (eh->h_dest, ethaddr,ETH_ALEN );memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 );/* XXX: not correct */ memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); eh->h_proto =htons (ETH_P_IP ); +break ; + } + +default : +/* Do not assert while we don't manage IP6VERSION */ +/* assert(0); */ +break ; + } + +memcpy (eh->h_dest, ethaddr,ETH_ALEN ); +DEBUG_ARGS ((dfd," src = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], + eh->h_source[3 ], eh->h_source[4 ], eh->h_source[5 ])); +DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], + eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ]));
Quoted 8 lines
slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 847
Quoted 5 lines
slirp/slirp.c +0
-1memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len);slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN );return 1 ; - } }The lines after the switch statement should also only be indented with 4 spaces, not with 8.
Ah, well, never mind, I did not see the comment in the patch description that you fix it up in the next patch.
Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe it would be nicer to do it in one go instead (after splitting off the arp_requested renaming)?
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 3 lines · Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote:
Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe it would be nicer to do it in one go instead (after splitting off the
arp_requestedrenaming)?
Please discuss about it with the people who asked for it. I won't revamp patches until what people prefer is settled.
Personally I prefer the two-patch approach, since the first nicely shows what behavior change we have, and the second is a no-op patch, as can be seen with diff -w
Samuel
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 7 lines · Samuel Thibault
Quoted 3 lines · Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote:
Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe it would be nicer to do it in one go instead (after splitting off the
arp_requestedrenaming)?Please discuss about it with the people who asked for it. I won't revamp patches until what people prefer is settled.
It's maybe best if Jan Kiszka could comment on such question since he's the slirp subsystem maintainer and finally has to decide whether to take the patches or not.
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
In reply to Thomas HuthQuoted 115 lines · Thomas Huth
Quoted 105 lines · Thomas Huth
Quoted 50 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
Basically, this patch replaces "arp" by "resolution" every time "arp" means "mac resolution" and not specifically ARP.
Some indentation problems are solved in functions that will be modified in the next patches (ip_input…).
In
if_encap, a switch is added to prepare for the IPv6 case. Some code is factorized.Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/mbuf.c | 2 +- slirp/mbuf.h | 2 +- slirp/slirp.c | 24 ++++++++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-)slirp/mbuf.c +1
-1@@ -91,7 +91,7 @@ m_get(Slirp *slirp) m->m_len =0 ; m->m_nextpkt =NULL ; m->m_prevpkt =NULL ; - m->arp_requested = false; + m->resolution_requested = false; m->expiration_date = (uint64_t )-1 ; end_error:DEBUG_ARG ("m = %p" , m);slirp/mbuf.h +1
-1@@ -79,7 +79,7 @@ struct mbuf {int m_len;/* Amount of data in this mbuf */ Slirp *slirp; -bool arp_requested; +bool resolution_requested;uint64_t expiration_date;/* start of dynamic buffer area, must be last element */ union {slirp/slirp.c +0
-0slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 783
Quoted 8 lines
slirp/slirp.c +2
-0@@ -776,6 +776,8 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)return 1 ; } +switch (iph->ip_v) { +case IPVERSION :if (iph->ip_dst.s_addr ==0 ) {/* 0.0.0.0 can not be a destination address, something went wrong, * avoid making it worse */ That indentation looks now broken - shouldn't the if-statement now be indented with four more spaces now?
Quoted 42 lines
slirp/slirp.c +18
-3@@ -786,7 +788,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)struct ethhdr *reh = (struct ethhdr *)arp_req;struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); -if (!ifm->arp_requested) { +if (!ifm->resolution_requested) {/* If the client addr is not known, send an ARP request */ memset (reh->h_dest,0xff ,ETH_ALEN );memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 );@@ -812,22 +814,36 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) rah->ar_tip = iph->ip_dst.s_addr; slirp->client_ipaddr = iph->ip_dst;slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); - ifm->arp_requested = true; + ifm->resolution_requested = true;/* Expire request and drop outgoing packet after 1 second */ ifm->expiration_date =qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; }return 0 ; }else { -memcpy (eh->h_dest, ethaddr,ETH_ALEN );memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 );/* XXX: not correct */ memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); eh->h_proto =htons (ETH_P_IP ); +break ; + } + +default : +/* Do not assert while we don't manage IP6VERSION */ +/* assert(0); */ +break ; + } + +memcpy (eh->h_dest, ethaddr,ETH_ALEN ); +DEBUG_ARGS ((dfd," src = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], + eh->h_source[3 ], eh->h_source[4 ], eh->h_source[5 ])); +DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], + eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ]));Quoted 8 lines
slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 847
Quoted 5 lines
slirp/slirp.c +0
-1memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len);slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN );return 1 ; - } }The lines after the switch statement should also only be indented with 4 spaces, not with 8.
Quoted 6 lines
Ah, well, never mind, I did not see the comment in the patch description that you fix it up in the next patch.
Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe it would be nicer to do it in one go instead (after splitting off the
arp_requestedrenaming)?
Or maybe even move the IPv4-only code into a separate function instead? ... otherwise if_encap() will be very huge once the IPv6 code has been added, I think.
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Just to explain.
I'm fine with revamping the patches, provided that we eventually converge somewhere.
What I'm afraid of is that splitting patches in yet more many pieces, revamping the code yet more (moving code into their own functions, etc.) will only make the patch series look even bigger and even less prone to be included for lack of reviewer time.
I agree that the presentation can be looked after. But if that makes the patch series much bigger, I'd really prefer to look after that once the content gets commited, otherwise I don't see how we'll ever manage to get this commited in the coming decade.
Samuel
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 10 lines · Samuel Thibault
Just to explain.
I'm fine with revamping the patches, provided that we eventually converge somewhere.
What I'm afraid of is that splitting patches in yet more many pieces, revamping the code yet more (moving code into their own functions, etc.) will only make the patch series look even bigger and even less prone to be included for lack of reviewer time.
Quoted 4 lines
I agree that the presentation can be looked after. But if that makes the patch series much bigger, I'd really prefer to look after that once the content gets commited, otherwise I don't see how we'll ever manage to get this commited in the coming decade.
Ok, point taken, and I understand your frustration about reworking and posting this a couple of times without getting the patches accepted.
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
In the end, it's up to the subsystem mainter how the patches should be done - so, Jan, what do you think? What way would you recommend to get the patches ready for being included?
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 5 lines · Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
Ok, that's what we already have: patches 1-9 are refactoring and support, and 10-18 are ipv6 code.
Samuel
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 9 lines · Samuel Thibault
Quoted 5 lines · Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
Ok, that's what we already have: patches 1-9 are refactoring and support, and 10-18 are ipv6 code.
Sounds good, ... then I'd suggest to sent the preparation patches separately next time and get them accepted first.
Thomas
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
meta
Quoted 8 lines · Thomas Huth
Quoted 7 lines · Samuel Thibault
Quoted 5 lines · Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
Quoted 5 lines
Quoted 2 lines
Ok, that's what we already have: patches 1-9 are refactoring and support, and 10-18 are ipv6 code.
Sounds good, ... then I'd suggest to sent the preparation patches separately next time and get them accepted first.
And then the next reviewer will say, "nice, but it would be even nicer to see what *motivates* these preparatory patches!" :)
Disclaimer: I don't have any technical context for this thread; I just noticed Samuel's email / frustration. I know that all too well, first hand, from this list and elsewhere. I don't know how it can be fixed, but I'm positive it is a *systemic* problem with this development model.
(I didn't contribute much value with this email, but perhaps Samuel will feel better by seeing some (however unsolicited) confirmation; plus hey it's Friday.)
Thanks
Laszlo
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 5 lines · Laszlo Ersek, on Fri 11 Dec 2015 16:40:32 +0100, wrote:
Quoted 2 lines
Sounds good, ... then I'd suggest to sent the preparation patches separately next time and get them accepted first.
And then the next reviewer will say, "nice, but it would be even nicer to see what *motivates* these preparatory patches!" :)
Yes, I also had that in mind, just didn't say it :)
Samuel
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
In reply to Laszlo ErsekQuoted 17 lines · Laszlo Ersek
meta
Quoted 8 lines · Thomas Huth
Quoted 7 lines · Samuel Thibault
Quoted 5 lines · Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
Quoted 5 lines
Quoted 2 lines
Ok, that's what we already have: patches 1-9 are refactoring and support, and 10-18 are ipv6 code.
Sounds good, ... then I'd suggest to sent the preparation patches separately next time and get them accepted first.
Quoted 7 lines
And then the next reviewer will say, "nice, but it would be even nicer to see what *motivates* these preparatory patches!" :)
Disclaimer: I don't have any technical context for this thread; I just noticed Samuel's email / frustration. I know that all too well, first hand, from this list and elsewhere. I don't know how it can be fixed, but I'm positive it is a *systemic* problem with this development model.
The best defense you have against this is to put more information in a commit message - any time you have split work to ease review, make sure to mention it in the commit message. Any time you choose to merge work into a single patch for less churn, mention it. The commit message is your greatest chance of explaining to reviewers WHY you did it the way you did; and a reasonable reviewer should be happy enough that you did the work without making you redo it to match their 'ivory tower' alternative approach that meets the same end. If you changed a patch from an earlier version due to a particular reviewer, feel free to mention that reviewer in your explanation of changes (although in this case, doing it in the cover letter or after the --- marker may be better than in the commit body proper).
Yes, I've also been on the receiving end of frustration of my patch not being perfect enough, and try to be relatively forgiving of different styles when they aren't outright forbidden by the code base's written standards (there's a huge difference between a patch not being technically correct, and merely not being stylistically ideal according to my ideals). Also, projects that automate standards checks (such as our scripts/checkpatch.pl) are nicer than ones that require contributors to comply with unwritten rules - but even then you can only encode so much into an automated checker, and still need to leave room for human judgment on when to bend the rules.
At any rate, if you ever feel like you are being asked to do too much churn for no real benefit, please call attention to that fact. Burn out is real, and suffering in silence is not beneficial to the project.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
Quoted 39 lines · Eric Blake
Quoted 17 lines · Laszlo Ersek
meta
Quoted 8 lines · Thomas Huth
Quoted 7 lines · Samuel Thibault
Quoted 5 lines · Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example make sense to split the whole series into two parts - first a series that does all the preparation and cleanup patches. And then once that has been reviewed and merged, send the second series that adds the real new IPv6 code.
Quoted 5 lines
Quoted 2 lines
Ok, that's what we already have: patches 1-9 are refactoring and support, and 10-18 are ipv6 code.
Sounds good, ... then I'd suggest to sent the preparation patches separately next time and get them accepted first.
Quoted 7 lines
And then the next reviewer will say, "nice, but it would be even nicer to see what *motivates* these preparatory patches!" :)
Disclaimer: I don't have any technical context for this thread; I just noticed Samuel's email / frustration. I know that all too well, first hand, from this list and elsewhere. I don't know how it can be fixed, but I'm positive it is a *systemic* problem with this development model.
The best defense you have against this is to put more information in a commit message - any time you have split work to ease review, make sure to mention it in the commit message. Any time you choose to merge work into a single patch for less churn, mention it. The commit message is your greatest chance of explaining to reviewers WHY you did it the way you did; and a reasonable reviewer should be happy enough that you did the work without making you redo it to match their 'ivory tower' alternative approach that meets the same end. If you changed a patch from an earlier version due to a particular reviewer, feel free to mention that reviewer in your explanation of changes (although in this case, doing it in the cover letter or after the --- marker may be better than in the commit body proper).
Quoted 6 lines
Yes, I've also been on the receiving end of frustration of my patch not being perfect enough, and try to be relatively forgiving of different styles when they aren't outright forbidden by the code base's written standards (there's a huge difference between a patch not being technically correct, and merely not being stylistically ideal according to my ideals).
I'm very impressed that you are conscious of this. I also seek to remind myself of it (in the reviewer role), frequently. Thank you.
Quoted 9 lines
Also, projects that automate standards checks (such as
ourscripts/checkpatch.pl) are nicer than ones that require contributors to comply with unwritten rules - but even then you can only encode so much into an automated checker, and still need to leave room for human judgment on when to bend the rules.At any rate, if you ever feel like you are being asked to do too much churn for no real benefit, please call attention to that fact. Burn out is real, and suffering in silence is not beneficial to the project.
I used to complain very loudly ("suffering in silence" is not what I'm naturally inclined to do :)), but recently I've had zero problems on qemu-devel, luckily. My only reason to go meta here was Samuel's emails, which looked all too familiar to my own earlier ones.
(This is not to say that I take issue with whatever Thomas or other reviewers may have said -- I have no clue what they said; I just noticed the "pattern" in Samuel's emails.)
Thanks!
Laszlo
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
In reply to Thomas HuthQuoted 8 lines · Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote:
slirp/slirp.c int if_encap(Slirp *slirp, struct mbuf *ifm) · line 783
Quoted 5 lines
slirp/slirp.c +2
-0+switch (iph->ip_v) { +case IPVERSION :if (iph->ip_dst.s_addr ==0 ) {/* 0.0.0.0 can not be a destination address, something went wrong, * avoid making it worse */ That indentation looks now broken - shouldn't the if-statement now be indented with four more spaces now?
Please see the patch summary: I was asked to provided a non-reindented patch and then a reindent patch.
Quoted 4 lines
Also, I'd prefer if you could split this patch into two: One for renaming the "
arp_requested" field, and one for adding the additional logic with the switch statement (since there are two different kind of changes).
TBH I'm starting to again lose motivation.
Samuel
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff
In reply to Thomas HuthQuoted 4 lines · Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote:
Also, I'd prefer if you could split this patch into two: One for renaming the "
arp_requested" field, and one for adding the additional logic with the switch statement (since there are two different kind of changes).
Ok, at least it's easy enough :)
Samuel
PATCH 3/18 slirp: Reindent after refactoring
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
No code change.
--- slirp/slirp.c | 117 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 59 insertions(+), 58 deletions(-)
slirp/slirp.c +59 -58
@@ -778,55 +778,56 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)switch (iph->ip_v) {case IPVERSION : -if (iph->ip_dst.s_addr ==0 ) { -/* 0.0.0.0 can not be a destination address, something went wrong, -* avoid making it worse */ -return 1 ; - } -if (!arp_table_search (slirp, iph->ip_dst.s_addr, ethaddr)) { -uint8_t arp_req[ETH_HLEN +sizeof (struct arphdr )]; -struct ethhdr *reh = (struct ethhdr *)arp_req; -struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); - -if (!ifm->resolution_requested) { -/* If the client addr is not known, send an ARP request */ -memset (reh->h_dest,0xff ,ETH_ALEN ); -memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 ); -memcpy (&reh->h_source[2 ], &slirp->vhost_addr,4 ); - reh->h_proto =htons (ETH_P_ARP ); - rah->ar_hrd =htons (1 ); - rah->ar_pro =htons (ETH_P_IP ); - rah->ar_hln =ETH_ALEN ; - rah->ar_pln =4 ; - rah->ar_op =htons (ARPOP_REQUEST ); - -/* source hw addr */ -memcpy (rah->ar_sha, special_ethaddr,ETH_ALEN -4 ); -memcpy (&rah->ar_sha[2 ], &slirp->vhost_addr,4 ); - -/* source IP */ - rah->ar_sip = slirp->vhost_addr.s_addr; - -/* target hw addr (none) */ -memset (rah->ar_tha,0 ,ETH_ALEN ); - -/* target IP */ - rah->ar_tip = iph->ip_dst.s_addr; - slirp->client_ipaddr = iph->ip_dst; -slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); - ifm->resolution_requested = true; - -/* Expire request and drop outgoing packet after 1 second */ - ifm->expiration_date =qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; +if (iph->ip_dst.s_addr ==0 ) { +/* 0.0.0.0 can not be a destination address, something went wrong, +* avoid making it worse */ +return 1 ; + } +if (!arp_table_search (slirp, iph->ip_dst.s_addr, ethaddr)) { +uint8_t arp_req[ETH_HLEN +sizeof (struct arphdr )]; +struct ethhdr *reh = (struct ethhdr *)arp_req; +struct arphdr *rah = (struct arphdr *)(arp_req +ETH_HLEN ); + +if (!ifm->resolution_requested) { +/* If the client addr is not known, send an ARP request */ +memset (reh->h_dest,0xff ,ETH_ALEN ); +memcpy (reh->h_source, special_ethaddr,ETH_ALEN -4 ); +memcpy (&reh->h_source[2 ], &slirp->vhost_addr,4 ); + reh->h_proto =htons (ETH_P_ARP ); + rah->ar_hrd =htons (1 ); + rah->ar_pro =htons (ETH_P_IP ); + rah->ar_hln =ETH_ALEN ; + rah->ar_pln =4 ; + rah->ar_op =htons (ARPOP_REQUEST ); + +/* source hw addr */ +memcpy (rah->ar_sha, special_ethaddr,ETH_ALEN -4 ); +memcpy (&rah->ar_sha[2 ], &slirp->vhost_addr,4 ); + +/* source IP */ + rah->ar_sip = slirp->vhost_addr.s_addr; + +/* target hw addr (none) */ +memset (rah->ar_tha,0 ,ETH_ALEN ); + +/* target IP */ + rah->ar_tip = iph->ip_dst.s_addr; + slirp->client_ipaddr = iph->ip_dst; +slirp_output (slirp->opaque, arp_req,sizeof (arp_req)); + ifm->resolution_requested = true; + +/* Expire request and drop outgoing packet after 1 second */ + ifm->expiration_date = +qemu_clock_get_ns (QEMU_CLOCK_REALTIME ) +1000000000ULL ; + } +return 0 ; + }else { +memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 ); +/* XXX: not correct */ +memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); + eh->h_proto =htons (ETH_P_IP ); +break ; } -return 0 ; - }else { -memcpy (eh->h_source, special_ethaddr,ETH_ALEN -4 ); -/* XXX: not correct */ -memcpy (&eh->h_source[2 ], &slirp->vhost_addr,4 ); - eh->h_proto =htons (ETH_P_IP ); -break ; - }default :/* Do not assert while we don't manage IP6VERSION */
@@ -834,16 +835,16 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)break ; } -memcpy (eh->h_dest, ethaddr,ETH_ALEN ); -DEBUG_ARGS ((dfd," src = %02x:%02x:%02x:%02x:%02x:%02x\n" , - eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], - eh->h_source[3 ], eh->h_source[4 ], eh->h_source[5 ])); -DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , - eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], - eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ])); -memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len); -slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN ); -return 1 ; +memcpy (eh->h_dest, ethaddr,ETH_ALEN ); +DEBUG_ARGS ((dfd," src = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_source[0 ], eh->h_source[1 ], eh->h_source[2 ], + eh->h_source[3 ], eh->h_source[4 ], eh->h_source[5 ])); +DEBUG_ARGS ((dfd," dst = %02x:%02x:%02x:%02x:%02x:%02x\n" , + eh->h_dest[0 ], eh->h_dest[1 ], eh->h_dest[2 ], + eh->h_dest[3 ], eh->h_dest[4 ], eh->h_dest[5 ])); +memcpy (buf +sizeof (struct ethhdr ), ifm->m_data, ifm->m_len); +slirp_output (slirp->opaque, buf, ifm->m_len +ETH_HLEN ); +return 1 ; }/* Drop host forwarding rule, return 0 if found. */
Signature · 1 line
2.6.2
PATCH 4/18 slirp: Make Socket structure IPv6 compatible
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch replaces foreign and local address/port couples in Socket structure by 2 sockaddr_storage which can be casted in sockaddr_in. Direct access to address and port is still possible thanks to some \#define, so retrocompatibility of the existing code is assured.
The ss_family field of sockaddr_storage is declared after each socket creation.
The whole structure is also saved/restored when a Qemu session is saved/restored.
--- slirp/ip_icmp.c | 2 ++ slirp/slirp.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- slirp/socket.c | 14 +++++++++++--- slirp/socket.h | 19 +++++++++++++++---- slirp/tcp_input.c | 2 ++ slirp/tcp_subr.c | 2 ++ slirp/udp.c | 4 ++++ 7 files changed, 76 insertions(+), 15 deletions(-)
slirp/ip_icmp.c +2 -0
@@ -170,8 +170,10 @@ icmp_input(struct mbuf *m, int hlen) goto end_error; } so->so_m = m; + so->so_ffamily =AF_INET ; so->so_faddr = ip->ip_dst; so->so_fport =htons (7 ); + so->so_lfamily =AF_INET ; so->so_laddr = ip->ip_src; so->so_lport =htons (9 ); so->so_iptos = ip->ip_tos;
slirp/slirp.c +40 -8
60 lines not on this page · raw message
slirp/socket.c +11 -3
@@ -437,8 +437,8 @@ sowrite(struct socket *so)void sorecvfrom (struct socket *so) { -struct sockaddr_in addr; -socklen_t addrlen =sizeof (struct sockaddr_in ); +struct sockaddr_storage addr; +socklen_t addrlen =sizeof (struct sockaddr_storage );DEBUG_CALL ("sorecvfrom" );DEBUG_ARG ("so = %p" , so);
@@ -527,7 +527,13 @@ sorecvfrom(struct socket *so)* If this packet was destined for CTL_ADDR, * make it look like that's where it came from, done by udp_output */ -udp_output (so, m, &addr); +switch (so->so_ffamily) { +case AF_INET : +udp_output (so, m, (struct sockaddr_in *) &addr); +break ; +default : +break ; + } }/* rx error */ }/* if ping packet */ }
@@ -619,6 +625,7 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, so->so_state &=SS_PERSISTENT_MASK ; so->so_state |= (SS_FACCEPTCONN | flags); + so->so_lfamily =AF_INET ; so->so_lport = lport;/* Kept in network format */ so->so_laddr.s_addr = laddr;/* Ditto */
@@ -645,6 +652,7 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (int ));getsockname (s,(struct sockaddr *)&addr,&addrlen); + so->so_ffamily =AF_INET ; so->so_fport = addr.sin_port;if (addr.sin_addr.s_addr ==0 || addr.sin_addr.s_addr == loopback_addr.s_addr) so->so_faddr = slirp->vhost_addr;
slirp/socket.h +15 -4
@@ -31,10 +31,21 @@ struct socket {struct tcpiphdr *so_ti;/* Pointer to the original ti within * so_mconn, for non-blocking connections */ int so_urgc; -struct in_addr so_faddr;/* foreign host table entry */ -struct in_addr so_laddr;/* local host table entry */ -uint16_t so_fport;/* foreign port */ -uint16_t so_lport;/* local port */ +union {/* foreign host */ +struct sockaddr_storage ss; +struct sockaddr_in sin; + } fhost; +#define so_faddr fhost.sin.sin_addr +#define so_fport fhost.sin.sin_port +#define so_ffamily fhost.ss.ss_family + +union {/* local host */ +struct sockaddr_storage ss; +struct sockaddr_in sin; + } lhost; +#define so_laddr lhost.sin.sin_addr +#define so_lport lhost.sin.sin_port +#define so_lfamily lhost.ss.ss_familyuint8_t so_iptos;/* Type of service */ uint8_t so_emu;/* Is the socket emulated? */
slirp/tcp_input.c +2 -0
@@ -374,8 +374,10 @@ findso:sbreserve (&so->so_snd,TCP_SNDSPACE );sbreserve (&so->so_rcv,TCP_RCVSPACE ); + so->so_lfamily =AF_INET ; so->so_laddr = ti->ti_src; so->so_lport = ti->ti_sport; + so->so_ffamily =AF_INET ; so->so_faddr = ti->ti_dst; so->so_fport = ti->ti_dport;
slirp/tcp_subr.c +2 -0
@@ -413,6 +413,7 @@ void tcp_connect(struct socket *inso)free (so);/* NOT sofree */ return ; } + so->so_lfamily =AF_INET ; so->so_laddr = inso->so_laddr; so->so_lport = inso->so_lport; }
@@ -430,6 +431,7 @@ void tcp_connect(struct socket *inso)qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (int ));socket_set_nodelay (s); + so->so_ffamily =AF_INET ; so->so_fport = addr.sin_port; so->so_faddr = addr.sin_addr;/* Translate connections from localhost to the real hostname */
slirp/udp.c +4 -0
@@ -190,6 +190,7 @@ udp_input(register struct mbuf *m, int iphlen)/* * Setup fields */ + so->so_lfamily =AF_INET ; so->so_laddr = ip->ip_src; so->so_lport = uh->uh_sport;
@@ -202,6 +203,7 @@ udp_input(register struct mbuf *m, int iphlen)*/ } + so->so_ffamily =AF_INET ; so->so_faddr = ip->ip_dst;/* XXX */ so->so_fport = uh->uh_dport;/* XXX */
@@ -376,6 +378,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,socket_set_fast_reuse (so->s);getsockname (so->s,(struct sockaddr *)&addr,&addrlen); + so->so_ffamily =AF_INET ; so->so_fport = addr.sin_port;if (addr.sin_addr.s_addr ==0 || addr.sin_addr.s_addr == loopback_addr.s_addr) {
@@ -383,6 +386,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, }else { so->so_faddr = addr.sin_addr; } + so->so_lfamily =AF_INET ; so->so_lport = lport; so->so_laddr.s_addr = laddr;if (flags !=SS_FACCEPTONCE )
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 04/18] slirp: Make Socket structure IPv6 compatible
Quoted 16 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
This patch replaces foreign and local address/port couples in Socket structure by 2
sockaddr_storagewhich can be casted insockaddr_in. Direct access to address and port is still possible thanks to some \#define, so retrocompatibility of the existing code is assured.The
ss_familyfield ofsockaddr_storageis declared after each socket creation.The whole structure is also saved/restored when a Qemu session is saved/restored.
Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>---
[...]
Quoted 53 lines
slirp/slirp.c +30
-8@@ -1028,10 +1028,26 @@ static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)static void slirp_socket_save (QEMUFile *f,struct socket *so) {qemu_put_be32 (f, so->so_urgc); -qemu_put_be32 (f, so->so_faddr.s_addr); -qemu_put_be32 (f, so->so_laddr.s_addr); -qemu_put_be16 (f, so->so_fport); -qemu_put_be16 (f, so->so_lport); +qemu_put_be16 (f, so->so_ffamily); +switch (so->so_ffamily) { +case AF_INET : +qemu_put_be32 (f, so->so_faddr.s_addr); +qemu_put_be16 (f, so->so_fport); +break ; +default : +fprintf (stderr, +"so_ffamily unknown, unable to save so_faddr and so_fport\n" ); + } +qemu_put_be16 (f, so->so_lfamily); +switch (so->so_lfamily) { +case AF_INET : +qemu_put_be32 (f, so->so_laddr.s_addr); +qemu_put_be16 (f, so->so_lport); +break ; +default : +fprintf (stderr, +"so_ffamily unknown, unable to save so_laddr and so_lport\n" ); + }qemu_put_byte (f, so->so_iptos);qemu_put_byte (f, so->so_emu);qemu_put_byte (f, so->so_type);@@ -1151,10 +1167,26 @@ static int slirp_socket_load(QEMUFile *f, struct socket *so)return -ENOMEM ; so->so_urgc =qemu_get_be32 (f); - so->so_faddr.s_addr =qemu_get_be32 (f); - so->so_laddr.s_addr =qemu_get_be32 (f); - so->so_fport =qemu_get_be16 (f); - so->so_lport =qemu_get_be16 (f); + so->so_ffamily =qemu_get_be16 (f); +switch (so->so_ffamily) { +case AF_INET : + so->so_faddr.s_addr =qemu_get_be32 (f); + so->so_fport =qemu_get_be16 (f); +break ; +default : +fprintf (stderr, +"so_ffamily unknown, unable to restore so_faddr and so_lport\n" ); + }
slirp/slirp.c static int slirp_socket_load(QEMUFile *f, struct socket *so) · line 1189
Quoted 10 lines
slirp/slirp.c +10
-0+ so->so_lfamily =qemu_get_be16 (f); +switch (so->so_lfamily) { +case AF_INET : + so->so_laddr.s_addr =qemu_get_be32 (f); + so->so_lport =qemu_get_be16 (f); +break ; +default : +fprintf (stderr, +"so_ffamily unknown, unable to restore so_laddr and so_lport\n" ); + }
Since you're changing the layout of the save data here, I think you have to bump the version_id number in the register_savevm() call in slirp_init() to make it clear that the new layout is incompatible.
slirp/socket.c
Quoted 7 lines
slirp/slirp.c +0
-0so->so_iptos =qemu_get_byte (f); so->so_emu =qemu_get_byte (f); so->so_type =qemu_get_byte (f);slirp/socket.c +0
-0
[...]
Remaining parts of the patch look fine to me.
Thomas
PATCH 5/18 slirp: Factorizing address translation
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch factorizes some duplicate code into a new function, sotranslate_out(). This function perform the address translation when a packet is transmitted to the host network. If the paquet is destinated to the host, the loopback address is used, and if the paquet is destinated to the virtual DNS, the real DNS address is used. This code is just a copy of the existant, but factorized and ready to manage the IPv6 case.
On the same model, the major part of udp_output() code is moved into a new sotranslate_in(). This function is directly used in sorecvfrom(), like sotranslate_out() in sosendto().udp_output() becoming useless, it is removed and udp_output2() is renamed into udp_output(). This adds consistency with the udp6_output() function introduced by further patches.
Lastly, this factorizes some duplicate code into sotranslate_accept(), which performs the address translation when a connection is established on the host for port forwarding: if it comes from localhost, the host virtual address is used instead.
--- slirp/bootp.c | 2 +- slirp/ip_icmp.c | 19 ++------- slirp/socket.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------- slirp/socket.h | 5 +++ slirp/tcp_subr.c | 35 ++++------------- slirp/tftp.c | 6 +-- slirp/udp.c | 37 ++---------------- slirp/udp.h | 3 +- 8 files changed, 119 insertions(+), 103 deletions(-)
slirp/bootp.c +1 -1
@@ -325,7 +325,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) m->m_len =sizeof (struct bootp_t ) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); }void bootp_input (struct mbuf *m)
slirp/ip_icmp.c +4 -15
@@ -157,7 +157,7 @@ icmp_input(struct mbuf *m, int hlen) goto freeit; }else {struct socket *so; -struct sockaddr_in addr; +struct sockaddr_storage addr;if ((so =socreate (slirp)) ==NULL ) goto freeit;if (icmp_send (so, m, hlen) ==0 ) {return ;
@@ -181,20 +181,9 @@ icmp_input(struct mbuf *m, int hlen) so->so_state =SS_ISFCONNECTED ;/* Send the packet */ - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else { - addr.sin_addr = so->so_faddr; - } - addr.sin_port = so->so_fport; + addr = so->fhost.ss; +sotranslate_out (so, &addr); +if (sendto (so->s, icmp_ping_msg,strlen (icmp_ping_msg),0 , (struct sockaddr *)&addr,sizeof (addr)) ==-1 ) { DEBUG_MISC((dfd,"icmp_input udp sendto tx errno = %d-%s\n",
slirp/socket.c +95 -20
148 lines not on this page · raw message
slirp/socket.h +5 -0
@@ -105,4 +105,9 @@ struct iovec; /* For win32 */size_t sopreprbuf (struct socket *so,struct iovec *iov,int *np);int soreadbuf (struct socket *so,const char *buf,int size); +void sotranslate_out (struct socket *,struct sockaddr_storage *); +void sotranslate_in (struct socket *,struct sockaddr_storage *); +void sotranslate_accept (struct socket *); + +#endif /* _SOCKET_H_ */
slirp/tcp_subr.c +7 -28
@@ -326,7 +326,6 @@ tcp_sockclosed(struct tcpcb *tp)*/ int tcp_fconnect (struct socket *so) { -Slirp *slirp = so->slirp;int ret=0 ;DEBUG_CALL ("tcp_fconnect" );
@@ -334,30 +333,17 @@ int tcp_fconnect(struct socket *so)if ( (ret = so->s =qemu_socket (AF_INET ,SOCK_STREAM ,0 )) >=0 ) {int opt, s=so->s; -struct sockaddr_in addr; +struct sockaddr_storage addr;qemu_set_nonblock (s);socket_set_fast_reuse (s); opt =1 ;qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (opt)); - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else - addr.sin_addr = so->so_faddr; - addr.sin_port = so->so_fport; - -DEBUG_MISC ((dfd," connect()ing, addr.sin_port=%d, " -"addr.sin_addr.s_addr=%.16s\n" , -ntohs (addr.sin_port),inet_ntoa (addr.sin_addr))); + addr = so->fhost.ss; + DEBUG_CALL(" connect()ing") +sotranslate_out (so, &addr); +/* We don't care what port we get */ ret =connect (s,(struct sockaddr *)&addr,sizeof (addr));
@@ -431,15 +417,8 @@ void tcp_connect(struct socket *inso)qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (int ));socket_set_nodelay (s); - so->so_ffamily =AF_INET ; - so->so_fport = addr.sin_port; - so->so_faddr = addr.sin_addr; -/* Translate connections from localhost to the real hostname */ -if (so->so_faddr.s_addr ==0 || - (so->so_faddr.s_addr & loopback_mask) == - (loopback_addr.s_addr & loopback_mask)) { - so->so_faddr = slirp->vhost_addr; - } + so->fhost.sin = addr; +sotranslate_accept (so);/* Close the accept() socket, set right state */ if (inso->so_state &SS_FACCEPTONCE ) {
slirp/tftp.c +3 -3
@@ -155,7 +155,7 @@ static int tftp_send_oack(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) -514 + n -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY );return 0 ; }
@@ -193,7 +193,7 @@ static void tftp_send_error(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) -514 +3 +strlen (msg) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); out:tftp_session_terminate (spt);
@@ -243,7 +243,7 @@ static void tftp_send_next_block(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) - (512 - nobytes) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY );if (nobytes ==512 ) {tftp_session_update (spt);
slirp/udp.c +3 -34
@@ -236,7 +236,7 @@ bad:m_free (m); } -int udp_output2 (struct socket *so,struct mbuf *m, +int udp_output (struct socket *so,struct mbuf *m,struct sockaddr_in *saddr,struct sockaddr_in *daddr,int iptos) {
@@ -287,31 +287,6 @@ int udp_output2(struct socket *so, struct mbuf *m,return (error); } -int udp_output (struct socket *so,struct mbuf *m, -struct sockaddr_in *addr) - -{ -Slirp *slirp = so->slirp; -struct sockaddr_in saddr, daddr; - - saddr = *addr; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; - -if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { - saddr.sin_addr = slirp->vhost_addr; - }else if (addr->sin_addr.s_addr == loopback_addr.s_addr || - so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { - saddr.sin_addr = so->so_faddr; - } - } - daddr.sin_addr = so->so_laddr; - daddr.sin_port = so->so_lport; - -return udp_output2 (so, m, &saddr, &daddr, so->so_iptos); -} -int udp_attach (struct socket *so) {
@@ -378,14 +353,8 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,socket_set_fast_reuse (so->s);getsockname (so->s,(struct sockaddr *)&addr,&addrlen); - so->so_ffamily =AF_INET ; - so->so_fport = addr.sin_port; -if (addr.sin_addr.s_addr ==0 || - addr.sin_addr.s_addr == loopback_addr.s_addr) { - so->so_faddr = slirp->vhost_addr; - }else { - so->so_faddr = addr.sin_addr; - } + so->fhost.sin = addr; +sotranslate_accept (so); so->so_lfamily =AF_INET ; so->so_lport = lport; so->so_laddr.s_addr = laddr;
slirp/udp.h +1 -2
@@ -76,12 +76,11 @@ struct mbuf;void udp_init (Slirp *);void udp_cleanup (Slirp *);void udp_input (registerstruct mbuf *,int ); -int udp_output (struct socket *,struct mbuf *,struct sockaddr_in *);int udp_attach (struct socket *);void udp_detach (struct socket *);struct socket *udp_listen (Slirp *,uint32_t ,u_int ,uint32_t ,u_int ,int ); -int udp_output2 (struct socket *so,struct mbuf *m, +int udp_output (struct socket *so,struct mbuf *m,struct sockaddr_in *saddr,struct sockaddr_in *daddr,int iptos);#endif
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 05/18] slirp: Factorizing address translation
Hello,
Thomas, this is the last refactoring patch which doesn't have a review yet, right?
Samuel
Quoted 435 lines · Samuel Thibault, on Fri 11 Dec 2015 01:15:17 +0100, wrote:
From: Guillaume Subiron <[email protected]>
This patch factorizes some duplicate code into a new function,
sotranslate_out(). This function perform the address translation when a packet is transmitted to the host network. If the paquet is destinated to the host, the loopback address is used, and if the paquet is destinated to the virtual DNS, the real DNS address is used. This code is just a copy of the existant, but factorized and ready to manage the IPv6 case.On the same model, the major part of
udp_output()code is moved into a newsotranslate_in(). This function is directly used insorecvfrom(), likesotranslate_out()insosendto().udp_output()becoming useless, it is removed andudp_output2()is renamed intoudp_output(). This adds consistency with theudp6_output()function introduced by further patches.Lastly, this factorizes some duplicate code into
sotranslate_accept(), which performs the address translation when a connection is established on the host for port forwarding: if it comes from localhost, the host virtual address is used instead.Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/bootp.c | 2 +- slirp/ip_icmp.c | 19 ++------- slirp/socket.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---------- slirp/socket.h | 5 +++ slirp/tcp_subr.c | 35 ++++------------- slirp/tftp.c | 6 +-- slirp/udp.c | 37 ++---------------- slirp/udp.h | 3 +- 8 files changed, 119 insertions(+), 103 deletions(-)slirp/bootp.c +1
-1@@ -325,7 +325,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) m->m_len =sizeof (struct bootp_t ) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); }void bootp_input (struct mbuf *m)slirp/ip_icmp.c +4
-15@@ -157,7 +157,7 @@ icmp_input(struct mbuf *m, int hlen) goto freeit; }else {struct socket *so; -struct sockaddr_in addr; +struct sockaddr_storage addr;if ((so =socreate (slirp)) ==NULL ) goto freeit;if (icmp_send (so, m, hlen) ==0 ) {return ;@@ -181,20 +181,9 @@ icmp_input(struct mbuf *m, int hlen) so->so_state =SS_ISFCONNECTED ;/* Send the packet */ - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else { - addr.sin_addr = so->so_faddr; - } - addr.sin_port = so->so_fport; + addr = so->fhost.ss; +sotranslate_out (so, &addr); +if (sendto (so->s, icmp_ping_msg,strlen (icmp_ping_msg),0 , (struct sockaddr *)&addr,sizeof (addr)) ==-1 ) { DEBUG_MISC((dfd,"icmp_input udp sendto tx errno = %d-%s\n",slirp/socket.c +95
-20@@ -438,6 +438,7 @@ void sorecvfrom(struct socket *so) {struct sockaddr_storage addr; +struct sockaddr_storage saddr, daddr;socklen_t addrlen =sizeof (struct sockaddr_storage );DEBUG_CALL ("sorecvfrom" );@@ -525,11 +526,17 @@ sorecvfrom(struct socket *so)/* * If this packet was destined for CTL_ADDR, -* make it look like that's where it came from, done by udp_output +* make it look like that's where it came from */ + saddr = addr; +sotranslate_in (so, &saddr); + daddr = so->lhost.ss; +switch (so->so_ffamily) {case AF_INET : -udp_output (so, m, (struct sockaddr_in *) &addr); +udp_output (so, m, (struct sockaddr_in *) &saddr, + (struct sockaddr_in *) &daddr, + so->so_iptos);break ;default :break ;@@ -544,33 +551,20 @@ sorecvfrom(struct socket *so)int sosendto (struct socket *so,struct mbuf *m) { -Slirp *slirp = so->slirp;int ret; -struct sockaddr_in addr; +struct sockaddr_storage addr;DEBUG_CALL ("sosendto" );DEBUG_ARG ("so = %p" , so);DEBUG_ARG ("m = %p" , m); - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else - addr.sin_addr = so->so_faddr; - addr.sin_port = so->so_fport; - -DEBUG_MISC ((dfd," sendto()ing, addr.sin_port=%d, addr.sin_addr.s_addr=%.16s\n" ,ntohs (addr.sin_port),inet_ntoa (addr.sin_addr))); + addr = so->fhost.ss; +DEBUG_CALL (" sendto()ing)" ); +sotranslate_out (so, &addr);/* Don't care what port we get */ ret =sendto (so->s, m->m_data, m->m_len,0 , - (struct sockaddr *)&addr,sizeof (struct sockaddr )); + (struct sockaddr *)&addr,sizeof (addr));if (ret <0 )return -1 ;@@ -726,3 +720,84 @@ sofwdrain(struct socket *so)else sofcantsendmore (so); } + +/* +* Translate addr in host addr when it is a virtual address +* :TODO:maethor:130314: Manage IPv6 +*/ +void sotranslate_out (struct socket *so,struct sockaddr_storage *addr) +{ +Slirp *slirp = so->slirp; +struct sockaddr_in *sin = (struct sockaddr_in *)addr; + +switch (addr->ss_family) { +case AF_INET : +if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == + slirp->vnetwork_addr.s_addr) { +/* It's an alias */ +if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { +if (get_dns_addr (&sin->sin_addr) <0 ) { + sin->sin_addr = loopback_addr; + } + }else { + sin->sin_addr = loopback_addr; + } + } + +DEBUG_MISC ((dfd," addr.sin_port=%d, " +"addr.sin_addr.s_addr=%.16s\n" , +ntohs (sin->sin_port),inet_ntoa (sin->sin_addr))); +break ; + +default : +break ; + } +} + +/* :TODO:maethor:130314: IPv6 */ +void sotranslate_in (struct socket *so,struct sockaddr_storage *addr) +{ +Slirp *slirp = so->slirp; +struct sockaddr_in *sin = (struct sockaddr_in *)addr; + +switch (addr->ss_family) { +case AF_INET : +if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == + slirp->vnetwork_addr.s_addr) { +uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; + +if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { + sin->sin_addr = slirp->vhost_addr; + }else if (sin->sin_addr.s_addr == loopback_addr.s_addr || + so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { + sin->sin_addr = so->so_faddr; + } + } +break ; + +default : +break ; + } +} + +/* +* Translate connections from localhost to the real hostname +* :TODO: IPv6 +*/ +void sotranslate_accept (struct socket *so) +{ +Slirp *slirp = so->slirp; + +switch (so->so_ffamily) { +case AF_INET : +if (so->so_faddr.s_addr ==INADDR_ANY || + (so->so_faddr.s_addr & loopback_mask) == + (loopback_addr.s_addr & loopback_mask)) { + so->so_faddr = slirp->vhost_addr; + } +break ; + +default : +break ; + } +}slirp/socket.h +5
-0@@ -105,4 +105,9 @@ struct iovec; /* For win32 */size_t sopreprbuf (struct socket *so,struct iovec *iov,int *np);int soreadbuf (struct socket *so,const char *buf,int size); +void sotranslate_out (struct socket *,struct sockaddr_storage *); +void sotranslate_in (struct socket *,struct sockaddr_storage *); +void sotranslate_accept (struct socket *); + +#endif /* _SOCKET_H_ */ slirp/tcp_subr.c +7
-28@@ -326,7 +326,6 @@ tcp_sockclosed(struct tcpcb *tp)*/ int tcp_fconnect (struct socket *so) { -Slirp *slirp = so->slirp;int ret=0 ;DEBUG_CALL ("tcp_fconnect" );@@ -334,30 +333,17 @@ int tcp_fconnect(struct socket *so)if ( (ret = so->s =qemu_socket (AF_INET ,SOCK_STREAM ,0 )) >=0 ) {int opt, s=so->s; -struct sockaddr_in addr; +struct sockaddr_storage addr;qemu_set_nonblock (s);socket_set_fast_reuse (s); opt =1 ;qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (opt)); - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else - addr.sin_addr = so->so_faddr; - addr.sin_port = so->so_fport; - -DEBUG_MISC ((dfd," connect()ing, addr.sin_port=%d, " -"addr.sin_addr.s_addr=%.16s\n" , -ntohs (addr.sin_port),inet_ntoa (addr.sin_addr))); + addr = so->fhost.ss; + DEBUG_CALL(" connect()ing") +sotranslate_out (so, &addr); +/* We don't care what port we get */ ret =connect (s,(struct sockaddr *)&addr,sizeof (addr));@@ -431,15 +417,8 @@ void tcp_connect(struct socket *inso)qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (int ));socket_set_nodelay (s); - so->so_ffamily =AF_INET ; - so->so_fport = addr.sin_port; - so->so_faddr = addr.sin_addr; -/* Translate connections from localhost to the real hostname */ -if (so->so_faddr.s_addr ==0 || - (so->so_faddr.s_addr & loopback_mask) == - (loopback_addr.s_addr & loopback_mask)) { - so->so_faddr = slirp->vhost_addr; - } + so->fhost.sin = addr; +sotranslate_accept (so);/* Close the accept() socket, set right state */ if (inso->so_state &SS_FACCEPTONCE ) {slirp/tftp.c +3
-3@@ -155,7 +155,7 @@ static int tftp_send_oack(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) -514 + n -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY );return 0 ; }@@ -193,7 +193,7 @@ static void tftp_send_error(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) -514 +3 +strlen (msg) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); out:tftp_session_terminate (spt);@@ -243,7 +243,7 @@ static void tftp_send_next_block(struct tftp_session *spt, m->m_len =sizeof (struct tftp_t ) - (512 - nobytes) -sizeof (struct ip ) -sizeof (struct udphdr ); -udp_output2 (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY ); +udp_output (NULL , m, &saddr, &daddr,IPTOS_LOWDELAY );if (nobytes ==512 ) {tftp_session_update (spt);slirp/udp.c +3
-34@@ -236,7 +236,7 @@ bad:m_free (m); } -int udp_output2 (struct socket *so,struct mbuf *m, +int udp_output (struct socket *so,struct mbuf *m,struct sockaddr_in *saddr,struct sockaddr_in *daddr,int iptos) {@@ -287,31 +287,6 @@ int udp_output2(struct socket *so, struct mbuf *m,return (error); } -int udp_output (struct socket *so,struct mbuf *m, -struct sockaddr_in *addr) - -{ -Slirp *slirp = so->slirp; -struct sockaddr_in saddr, daddr; - - saddr = *addr; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; - -if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { - saddr.sin_addr = slirp->vhost_addr; - }else if (addr->sin_addr.s_addr == loopback_addr.s_addr || - so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { - saddr.sin_addr = so->so_faddr; - } - } - daddr.sin_addr = so->so_laddr; - daddr.sin_port = so->so_lport; - -return udp_output2 (so, m, &saddr, &daddr, so->so_iptos); -} -int udp_attach (struct socket *so) {@@ -378,14 +353,8 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,socket_set_fast_reuse (so->s);getsockname (so->s,(struct sockaddr *)&addr,&addrlen); - so->so_ffamily =AF_INET ; - so->so_fport = addr.sin_port; -if (addr.sin_addr.s_addr ==0 || - addr.sin_addr.s_addr == loopback_addr.s_addr) { - so->so_faddr = slirp->vhost_addr; - }else { - so->so_faddr = addr.sin_addr; - } + so->fhost.sin = addr; +sotranslate_accept (so); so->so_lfamily =AF_INET ; so->so_lport = lport; so->so_laddr.s_addr = laddr;slirp/udp.h +1
-2@@ -76,12 +76,11 @@ struct mbuf;void udp_init (Slirp *);void udp_cleanup (Slirp *);void udp_input (registerstruct mbuf *,int ); -int udp_output (struct socket *,struct mbuf *,struct sockaddr_in *);int udp_attach (struct socket *);void udp_detach (struct socket *);struct socket *udp_listen (Slirp *,uint32_t ,u_int ,uint32_t ,u_int ,int ); -int udp_output2 (struct socket *so,struct mbuf *m, +int udp_output (struct socket *so,struct mbuf *m,struct sockaddr_in *saddr,struct sockaddr_in *daddr,int iptos);#endif Signature · 1 line
2.6.2
Signature · 4 lines
Samuel <c> hiri, le cri ici, c des marrants <c> j'ai un rep ".uglyhackdirectorywithoutacls" ds mon home -+- #ens-mim en stage -+-
Thomas Huth
Reviewed-byRe: [Qemu-devel] [PATCH 05/18] slirp: Factorizing address translation
Quoted 4 lines · Samuel Thibault
Hello,
Thomas, this is the last refactoring patch which doesn't have a review yet, right?
Right. It somehow showed up out of order in my e-mail program, so I missed it on Friday, sorry. So here's a review...
Quoted 7 lines
Quoted 5 lines · Samuel Thibault, on Fri 11 Dec 2015 01:15:17 +0100, wrote:
From: Guillaume Subiron <[email protected]>
This patch factorizes some duplicate code into a new function,
sotranslate_out(). This function perform the address translation when a packet is transmitted to the host network. If the paquet is destinated
s/paquet/packet/ and s/destinated/destined/ (I think)
Quoted 3 lines
Quoted 3 lines
to the host, the loopback address is used, and if the paquet is destinated to the virtual DNS, the real DNS address is used. This code is just a copy of the existant, but factorized and ready to manage the
s/existant/existent/
Quoted 17 lines
Quoted 17 lines
IPv6 case.
On the same model, the major part of
udp_output()code is moved into a newsotranslate_in(). This function is directly used insorecvfrom(), likesotranslate_out()insosendto().udp_output()becoming useless, it is removed andudp_output2()is renamed intoudp_output(). This adds consistency with theudp6_output()function introduced by further patches.Lastly, this factorizes some duplicate code into
sotranslate_accept(), which performs the address translation when a connection is established on the host for port forwarding: if it comes from localhost, the host virtual address is used instead.Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>---
[...]
Quoted 71 lines
Quoted 71 lines
slirp/socket.c +14
-20@@ -438,6 +438,7 @@ void sorecvfrom(struct socket *so) {struct sockaddr_storage addr; +struct sockaddr_storage saddr, daddr;socklen_t addrlen =sizeof (struct sockaddr_storage );DEBUG_CALL ("sorecvfrom" );@@ -525,11 +526,17 @@ sorecvfrom(struct socket *so)/* * If this packet was destined for CTL_ADDR, -* make it look like that's where it came from, done by udp_output +* make it look like that's where it came from */ + saddr = addr; +sotranslate_in (so, &saddr); + daddr = so->lhost.ss; +switch (so->so_ffamily) {case AF_INET : -udp_output (so, m, (struct sockaddr_in *) &addr); +udp_output (so, m, (struct sockaddr_in *) &saddr, + (struct sockaddr_in *) &daddr, + so->so_iptos);break ;default :break ;@@ -544,33 +551,20 @@ sorecvfrom(struct socket *so)int sosendto (struct socket *so,struct mbuf *m) { -Slirp *slirp = so->slirp;int ret; -struct sockaddr_in addr; +struct sockaddr_storage addr;DEBUG_CALL ("sosendto" );DEBUG_ARG ("so = %p" , so);DEBUG_ARG ("m = %p" , m); - addr.sin_family =AF_INET ; -if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { -/* It's an alias */ -if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { -if (get_dns_addr (&addr.sin_addr) <0 ) - addr.sin_addr = loopback_addr; - }else { - addr.sin_addr = loopback_addr; - } - }else - addr.sin_addr = so->so_faddr; - addr.sin_port = so->so_fport; - -DEBUG_MISC ((dfd," sendto()ing, addr.sin_port=%d, addr.sin_addr.s_addr=%.16s\n" ,ntohs (addr.sin_port),inet_ntoa (addr.sin_addr))); + addr = so->fhost.ss; +DEBUG_CALL (" sendto()ing)" ); +sotranslate_out (so, &addr);/* Don't care what port we get */ ret =sendto (so->s, m->m_data, m->m_len,0 , - (struct sockaddr *)&addr,sizeof (struct sockaddr )); + (struct sockaddr *)&addr,sizeof (addr));if (ret <0 )return -1 ;
Quoted 8 lines
slirp/socket.c sofwdrain(struct socket *so) · line 726
Quoted 8 lines
slirp/socket.c +4
-0@@ -726,3 +720,84 @@ sofwdrain(struct socket *so)else sofcantsendmore (so); } + +/* +* Translate addr in host addr when it is a virtual address +* :TODO:maethor:130314: Manage IPv6
In case you rework this patch, I think you could remove the "TODO" ... that looks rather like an interim developer's comment that should not be in the final patch, I think.
Quoted 23 lines
Quoted 23 lines
slirp/socket.c +23
-0+*/ +void sotranslate_out (struct socket *so,struct sockaddr_storage *addr) +{ +Slirp *slirp = so->slirp; +struct sockaddr_in *sin = (struct sockaddr_in *)addr; + +switch (addr->ss_family) { +case AF_INET : +if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == + slirp->vnetwork_addr.s_addr) { +/* It's an alias */ +if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) { +if (get_dns_addr (&sin->sin_addr) <0 ) { + sin->sin_addr = loopback_addr; + } + }else { + sin->sin_addr = loopback_addr; + } + } + +DEBUG_MISC ((dfd," addr.sin_port=%d, " +"addr.sin_addr.s_addr=%.16s\n" , +ntohs (sin->sin_port),inet_ntoa (sin->sin_addr)));
Quoted 8 lines
slirp/socket.c sofwdrain(struct socket *so) · line 757
Quoted 8 lines
slirp/socket.c +8
-0+break ; + +default : +break ; + } +} + +/* :TODO:maethor:130314: IPv6 */
dito
Quoted 38 lines
Quoted 38 lines
slirp/socket.c +38
-0+void sotranslate_in (struct socket *so,struct sockaddr_storage *addr) +{ +Slirp *slirp = so->slirp; +struct sockaddr_in *sin = (struct sockaddr_in *)addr; + +switch (addr->ss_family) { +case AF_INET : +if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == + slirp->vnetwork_addr.s_addr) { +uint32_t inv_mask = ~slirp->vnetwork_mask.s_addr; + +if ((so->so_faddr.s_addr & inv_mask) == inv_mask) { + sin->sin_addr = slirp->vhost_addr; + }else if (sin->sin_addr.s_addr == loopback_addr.s_addr || + so->so_faddr.s_addr != slirp->vhost_addr.s_addr) { + sin->sin_addr = so->so_faddr; + } + } +break ; + +default : +break ; + } +} + +/* +* Translate connections from localhost to the real hostname +* :TODO: IPv6 +*/ +void sotranslate_accept (struct socket *so) +{ +Slirp *slirp = so->slirp; + +switch (so->so_ffamily) { +case AF_INET : +if (so->so_faddr.s_addr ==INADDR_ANY || + (so->so_faddr.s_addr & loopback_mask) == + (loopback_addr.s_addr & loopback_mask)) {
Quoted 8 lines
slirp/socket.c sofwdrain(struct socket *so) · line 803
Quoted 8 lines
slirp/socket.c +8
-0+ so->so_faddr = slirp->vhost_addr; + } +break ; + +default : +break ; + } +}
[...]
I'd maybe also split this up into several patches, one for sotranslate_out(), and one or two for the remaining changes, but it also looks fine to me as it currently is already.
PATCH 6/18 slirp: Factorizing and cleaning solookup()
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch makes solookup() compatible with varying address families. Also, this function was only compatible with TCP. Having the socket list in argument, it is now compatible with UDP too. Finally, some optimization code is factorized inside the function (the function look at the last returned result before browsing the complete socket list).
This also adds a sockaddr_equal() function to compare two sockaddr_storage.
--- slirp/socket.c | 30 ++++++++++++++++-------------- slirp/socket.h | 23 ++++++++++++++++++++++- slirp/tcp_input.c | 27 +++++++++++---------------- slirp/udp.c | 25 ++++++------------------- 4 files changed, 55 insertions(+), 50 deletions(-)
slirp/socket.c +16 -14
@@ -15,24 +15,26 @@static void sofcantrcvmore (struct socket *so);static void sofcantsendmore (struct socket *so); -struct socket * -solookup (struct socket *head,struct in_addr laddr,u_int lport, -struct in_addr faddr,u_int fport) +struct socket *solookup (struct socket **last,struct socket *head, +struct sockaddr_storage *lhost,struct sockaddr_storage *fhost) { -struct socket *so; +struct socket *so = *last; -for (so = head->so_next; so != head; so = so->so_next) { -if (so->so_lport == lport && - so->so_laddr.s_addr == laddr.s_addr && - so->so_faddr.s_addr == faddr.s_addr && - so->so_fport == fport) -break ; - } +/* Optimisation */ +if (so != head &&sockaddr_equal (&(so->lhost.ss), lhost) + && (!fhost ||sockaddr_equal (&(so->fhost.ss), fhost))) { +return so; + } -if (so == head) -return (struct socket *)NULL ; -return so; +for (so = head->so_next; so != head; so = so->so_next) { +if (sockaddr_equal (&(so->lhost.ss), lhost) + && (!fhost ||sockaddr_equal (&(so->fhost.ss), fhost))) { + *last = so; +return so; + } + } +return (struct socket *)NULL ; }/*
slirp/socket.h +22 -1
@@ -87,7 +87,28 @@ struct socket {#define SS_HOSTFWD 0x1000/* Socket describes host->guest forwarding */ #define SS_INCOMING 0x2000/* Connection was initiated by a host on the internet */ -struct socket *solookup (struct socket *,struct in_addr ,u_int ,struct in_addr ,u_int ); +static inline int sockaddr_equal (struct sockaddr_storage *a, +struct sockaddr_storage *b) +{ +if (a->ss_family != b->ss_family) { +return 0 ; + }else { +switch (a->ss_family) { +case AF_INET : + { +struct sockaddr_in *a4 = (struct sockaddr_in *) a; +struct sockaddr_in *b4 = (struct sockaddr_in *) b; +return (a4->sin_addr.s_addr == b4->sin_addr.s_addr + && a4->sin_port == b4->sin_port); + } +default : +assert (0 ); + } + } +} + +struct socket *solookup (struct socket **,struct socket *, +struct sockaddr_storage *,struct sockaddr_storage *);struct socket *socreate (Slirp *);void sofree (struct socket *);int soread (struct socket *);
slirp/tcp_input.c +11 -16
@@ -227,6 +227,7 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)int iss =0 ;u_long tiwin;int ret; +struct sockaddr_storage lhost, fhost;struct ex_list *ex_ptr;Slirp *slirp;
@@ -320,16 +321,14 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)* Locate pcb for segment. */ findso: - so = slirp->tcp_last_so; -if (so->so_fport != ti->ti_dport || - so->so_lport != ti->ti_sport || - so->so_laddr.s_addr != ti->ti_src.s_addr || - so->so_faddr.s_addr != ti->ti_dst.s_addr) { - so =solookup (&slirp->tcb, ti->ti_src, ti->ti_sport, - ti->ti_dst, ti->ti_dport); -if (so) - slirp->tcp_last_so = so; - } + lhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&lhost)->sin_addr = ti->ti_src; + ((struct sockaddr_in *)&lhost)->sin_port = ti->ti_sport; + fhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&fhost)->sin_addr = ti->ti_dst; + ((struct sockaddr_in *)&fhost)->sin_port = ti->ti_dport; + + so =solookup (&slirp->tcp_last_so, &slirp->tcb, &lhost, &fhost);/* * If the state is CLOSED (i.e., TCB does not exist) then
@@ -374,12 +373,8 @@ findso:sbreserve (&so->so_snd,TCP_SNDSPACE );sbreserve (&so->so_rcv,TCP_RCVSPACE ); - so->so_lfamily =AF_INET ; - so->so_laddr = ti->ti_src; - so->so_lport = ti->ti_sport; - so->so_ffamily =AF_INET ; - so->so_faddr = ti->ti_dst; - so->so_fport = ti->ti_dport; + so->lhost.ss = lhost; + so->fhost.ss = fhost;if ((so->so_iptos =tcp_tos (so)) ==0 ) so->so_iptos = ((struct ip *)ti)->ip_tos;
slirp/udp.c +6 -19
@@ -70,6 +70,7 @@ udp_input(register struct mbuf *m, int iphlen)int len;struct ip save_ip;struct socket *so; +struct sockaddr_storage lhost;DEBUG_CALL ("udp_input" );DEBUG_ARG ("m = %p" , m);
@@ -151,25 +152,11 @@ udp_input(register struct mbuf *m, int iphlen)/* * Locate pcb for datagram. */ - so = slirp->udp_last_so; -if (so == &slirp->udb || so->so_lport != uh->uh_sport || - so->so_laddr.s_addr != ip->ip_src.s_addr) { -struct socket *tmp; - -for (tmp = slirp->udb.so_next; tmp != &slirp->udb; - tmp = tmp->so_next) { -if (tmp->so_lport == uh->uh_sport && - tmp->so_laddr.s_addr == ip->ip_src.s_addr) { - so = tmp; -break ; - } - } -if (tmp == &slirp->udb) { - so =NULL ; - }else { - slirp->udp_last_so = so; - } - } + lhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&lhost)->sin_addr = ip->ip_src; + ((struct sockaddr_in *)&lhost)->sin_port = uh->uh_sport; + + so =solookup (&slirp->udp_last_so, &slirp->udb, &lhost,NULL );if (so ==NULL ) {/*
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()
Quoted 10 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
This patch makes
solookup()compatible with varying address families. Also, this function was only compatible with TCP. Having the socket list in argument, it is now compatible with UDP too. Finally, some optimization code is factorized inside the function (the function look at the last returned result before browsing the complete socket list).This also adds a
sockaddr_equal()function to compare twosockaddr_storage.
I'd maybe also split this patch into two - first introduce the sockaddr_equal() function, then do the other changes. If you do too much stuff in one patch, it gets more difficult to read.
Quoted 3 lines
Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>---
[...]
Quoted 18 lines
slirp/socket.h +9
-1@@ -87,7 +87,28 @@ struct socket {#define SS_HOSTFWD 0x1000/* Socket describes host->guest forwarding */ #define SS_INCOMING 0x2000/* Connection was initiated by a host on the internet */ -struct socket *solookup (struct socket *,struct in_addr ,u_int ,struct in_addr ,u_int ); +static inline int sockaddr_equal (struct sockaddr_storage *a, +struct sockaddr_storage *b) +{ +if (a->ss_family != b->ss_family) { +return 0 ; + }else { +switch (a->ss_family) { +case AF_INET : + {
slirp/socket.h struct socket { · line 108
Quoted 10 lines
slirp/socket.h +10
-0+struct sockaddr_in *a4 = (struct sockaddr_in *) a; +struct sockaddr_in *b4 = (struct sockaddr_in *) b; +return (a4->sin_addr.s_addr == b4->sin_addr.s_addr + && a4->sin_port == b4->sin_port); + } +default : +assert (0 ); + } + } +}
Since the first part of the if statement always returns, you could get rid of one level of indentation here by removing the "else".
Thomas
Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()
Quoted 9 lines · Thomas Huth, on Fri 11 Dec 2015 16:06:22 +0100, wrote:
Quoted 8 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
This patch makes
solookup()compatible with varying address families. Also, this function was only compatible with TCP. Having the socket list in argument, it is now compatible with UDP too. Finally, some optimization code is factorized inside the function (the function look at the last returned result before browsing the complete socket list).
Quoted 6 lines
Quoted 2 lines
This also adds a
sockaddr_equal()function to compare twosockaddr_storage.I'd maybe also split this patch into two - first introduce the
sockaddr_equal()function, then do the other changes. If you do too much stuff in one patch, it gets more difficult to read.
Actually I think I'll do the converse. solookup doesn't take a sockaddr first, so introducing sockaddr_equal just to make it take separate in_addr and lport would make both patches clumsy. Making solookup access sockaddr first is however not too bad.
Samuel
Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()
Quoted 13 lines · Samuel Thibault, on Fri 11 Dec 2015 20:29:44 +0100, wrote:
Quoted 12 lines · Thomas Huth, on Fri 11 Dec 2015 16:06:22 +0100, wrote:
Quoted 10 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
This patch makes
solookup()compatible with varying address families. Also, this function was only compatible with TCP. Having the socket list in argument, it is now compatible with UDP too. Finally, some optimization code is factorized inside the function (the function look at the last returned result before browsing the complete socket list).This also adds a
sockaddr_equal()function to compare twosockaddr_storage.
Quoted 8 lines
Quoted 3 lines
I'd maybe also split this patch into two - first introduce the
sockaddr_equal()function, then do the other changes. If you do too much stuff in one patch, it gets more difficult to read.Actually I think I'll do the converse. solookup doesn't take a sockaddr first, so introducing
sockaddr_equaljust to make it take separatein_addrand lport would make both patches clumsy. Making solookup access sockaddr first is however not too bad.
Mmm, that's still clumsy: making solookup use sockaddr_storage without using solookup means putting all the casts there, it's really unreadable, while the version with sockaddr_equal is nice to read. And conversely, introducing sockaddr_equal first is clumsy.
I'll however have a look at introducing the optimizations etc. first, so that both making solookup use sockaddr and introducing sockaddr_equal looks natural.
Samuel
Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()
Quoted 3 lines · Samuel Thibault, on Fri 11 Dec 2015 20:38:19 +0100, wrote:
I'll however have a look at introducing the optimizations etc. first, so that both making solookup use sockaddr and introducing
sockaddr_equallooks natural.
Yes, it seems to be working very nicely.
Samuel
Re: [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup()
Quoted 6 lines · Samuel Thibault, on Fri 11 Dec 2015 20:51:02 +0100, wrote:
Quoted 3 lines · Samuel Thibault, on Fri 11 Dec 2015 20:38:19 +0100, wrote:
I'll however have a look at introducing the optimizations etc. first, so that both making solookup use sockaddr and introducing
sockaddr_equallooks natural.Yes, it seems to be working very nicely.
Just FYI, here is how it looks like now.
Samuel
slirp/socket.c +24 -13
@@ -16,23 +16,34 @@ static void sofcantrcvmore(struct socket *so);static void sofcantsendmore (struct socket *so);struct socket * -solookup (struct socket *head,struct in_addr laddr,u_int lport, +solookup (struct socket **last,struct socket *head, +struct in_addr laddr,u_int lport,struct in_addr faddr,u_int fport) { -struct socket *so; - -for (so = head->so_next; so != head; so = so->so_next) { -if (so->so_lport == lport && - so->so_laddr.s_addr == laddr.s_addr && - so->so_faddr.s_addr == faddr.s_addr && - so->so_fport == fport) -break ; - } +struct socket *so = *last; + +/* Optimisation */ +if (so != head && + so->so_lport == lport && + so->so_laddr.s_addr == laddr.s_addr && + (!faddr.s_addr || + (so->so_faddr.s_addr == faddr.s_addr && + so->so_fport == fport))) { +return so; + } -if (so == head) -return (struct socket *)NULL ; -return so; +for (so = head->so_next; so != head; so = so->so_next) { +if (so->so_lport == lport && + so->so_laddr.s_addr == laddr.s_addr && + (!faddr.s_addr || + (so->so_faddr.s_addr == faddr.s_addr && + so->so_fport == fport))) { + *last = so; +return so; + } + } +return (struct socket *)NULL ; }/*
slirp/socket.h +2 -1
@@ -87,7 +87,8 @@ struct socket {#define SS_HOSTFWD 0x1000/* Socket describes host->guest forwarding */ #define SS_INCOMING 0x2000/* Connection was initiated by a host on the internet */ -struct socket *solookup (struct socket *,struct in_addr ,u_int ,struct in_addr ,u_int ); +struct socket *solookup (struct socket **,struct socket *, +struct in_addr ,u_int ,struct in_addr ,u_int );struct socket *socreate (Slirp *);void sofree (struct socket *);int soread (struct socket *);
slirp/tcp_input.c +3 -10
@@ -320,16 +320,9 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)* Locate pcb for segment. */ findso: - so = slirp->tcp_last_so; -if (so->so_fport != ti->ti_dport || - so->so_lport != ti->ti_sport || - so->so_laddr.s_addr != ti->ti_src.s_addr || - so->so_faddr.s_addr != ti->ti_dst.s_addr) { - so =solookup (&slirp->tcb, ti->ti_src, ti->ti_sport, - ti->ti_dst, ti->ti_dport); -if (so) - slirp->tcp_last_so = so; - } + so =solookup (&slirp->tcp_last_so, &slirp->tcb, + ti->ti_src, ti->ti_sport, + ti->ti_dst, ti->ti_dport);/* * If the state is CLOSED (i.e., TCB does not exist) then
slirp/udp.c +2 -19
@@ -151,25 +151,8 @@ udp_input(register struct mbuf *m, int iphlen)/* * Locate pcb for datagram. */ - so = slirp->udp_last_so; -if (so == &slirp->udb || so->so_lport != uh->uh_sport || - so->so_laddr.s_addr != ip->ip_src.s_addr) { -struct socket *tmp; - -for (tmp = slirp->udb.so_next; tmp != &slirp->udb; - tmp = tmp->so_next) { -if (tmp->so_lport == uh->uh_sport && - tmp->so_laddr.s_addr == ip->ip_src.s_addr) { - so = tmp; -break ; - } - } -if (tmp == &slirp->udb) { - so =NULL ; - }else { - slirp->udp_last_so = so; - } - } + so =solookup (&slirp->udp_last_so, &slirp->udb, + ip->ip_src, uh->uh_sport, (struct in_addr ) {0 },0 );if (so ==NULL ) {/*
slirp/socket.c +6 -15
@@ -15,29 +15,20 @@static void sofcantrcvmore (struct socket *so);static void sofcantsendmore (struct socket *so); -struct socket * -solookup (struct socket **last,struct socket *head, -struct in_addr laddr,u_int lport, -struct in_addr faddr,u_int fport) +struct socket *solookup (struct socket **last,struct socket *head, +struct sockaddr_storage *lhost,struct sockaddr_storage *fhost) {struct socket *so = *last;/* Optimisation */ -if (so != head && - so->so_lport == lport && - so->so_laddr.s_addr == laddr.s_addr && - (!faddr.s_addr || - (so->so_faddr.s_addr == faddr.s_addr && - so->so_fport == fport))) { +if (so != head &&sockaddr_equal (&(so->lhost.ss), lhost) + && (!fhost ||sockaddr_equal (&(so->fhost.ss), fhost))) {return so; }for (so = head->so_next; so != head; so = so->so_next) { -if (so->so_lport == lport && - so->so_laddr.s_addr == laddr.s_addr && - (!faddr.s_addr || - (so->so_faddr.s_addr == faddr.s_addr && - so->so_fport == fport))) { +if (sockaddr_equal (&(so->lhost.ss), lhost) + && (!fhost ||sockaddr_equal (&(so->fhost.ss), fhost))) { *last = so;return so; }
slirp/socket.h +21 -2
@@ -87,8 +87,27 @@ struct socket {#define SS_HOSTFWD 0x1000/* Socket describes host->guest forwarding */ #define SS_INCOMING 0x2000/* Connection was initiated by a host on the internet */ -struct socket *solookup (struct socket **,struct socket *, -struct in_addr ,u_int ,struct in_addr ,u_int ); +static inline int sockaddr_equal (struct sockaddr_storage *a, +struct sockaddr_storage *b) +{ +if (a->ss_family != b->ss_family) +return 0 ; + +switch (a->ss_family) { +case AF_INET : + { +struct sockaddr_in *a4 = (struct sockaddr_in *) a; +struct sockaddr_in *b4 = (struct sockaddr_in *) b; +return (a4->sin_addr.s_addr == b4->sin_addr.s_addr + && a4->sin_port == b4->sin_port); + } +default : +assert (0 ); + } +} + +struct socket *solookup (struct socket **,struct socket *, +struct sockaddr_storage *,struct sockaddr_storage *);struct socket *socreate (Slirp *);void sofree (struct socket *);int soread (struct socket *);
slirp/tcp_input.c +11 -9
@@ -227,6 +227,7 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)int iss =0 ;u_long tiwin;int ret; +struct sockaddr_storage lhost, fhost;struct ex_list *ex_ptr;Slirp *slirp;
@@ -320,9 +321,14 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)* Locate pcb for segment. */ findso: - so =solookup (&slirp->tcp_last_so, &slirp->tcb, - ti->ti_src, ti->ti_sport, - ti->ti_dst, ti->ti_dport); + lhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&lhost)->sin_addr = ti->ti_src; + ((struct sockaddr_in *)&lhost)->sin_port = ti->ti_sport; + fhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&fhost)->sin_addr = ti->ti_dst; + ((struct sockaddr_in *)&fhost)->sin_port = ti->ti_dport; + + so =solookup (&slirp->tcp_last_so, &slirp->tcb, &lhost, &fhost);/* * If the state is CLOSED (i.e., TCB does not exist) then
@@ -367,12 +373,8 @@ findso:sbreserve (&so->so_snd,TCP_SNDSPACE );sbreserve (&so->so_rcv,TCP_RCVSPACE ); - so->so_lfamily =AF_INET ; - so->so_laddr = ti->ti_src; - so->so_lport = ti->ti_sport; - so->so_ffamily =AF_INET ; - so->so_faddr = ti->ti_dst; - so->so_fport = ti->ti_dport; + so->lhost.ss = lhost; + so->fhost.ss = fhost;if ((so->so_iptos =tcp_tos (so)) ==0 ) so->so_iptos = ((struct ip *)ti)->ip_tos;
slirp/udp.c +6 -2
@@ -70,6 +70,7 @@ udp_input(register struct mbuf *m, int iphlen)int len;struct ip save_ip;struct socket *so; +struct sockaddr_storage lhost;DEBUG_CALL ("udp_input" );DEBUG_ARG ("m = %p" , m);
@@ -151,8 +152,11 @@ udp_input(register struct mbuf *m, int iphlen)/* * Locate pcb for datagram. */ - so =solookup (&slirp->udp_last_so, &slirp->udb, - ip->ip_src, uh->uh_sport, (struct in_addr ) {0 },0 ); + lhost.ss_family =AF_INET ; + ((struct sockaddr_in *)&lhost)->sin_addr = ip->ip_src; + ((struct sockaddr_in *)&lhost)->sin_port = uh->uh_sport; + + so =solookup (&slirp->udp_last_so, &slirp->udb, &lhost,NULL );if (so ==NULL ) {/*
PATCH 7/18 slirp: Make udp_attach IPv6 compatible
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
A sa_family_t is now passed in argument to udp_attach instead of using a hardcoded "AF_INET" to call qemu_socket().
--- slirp/ip_icmp.c | 2 +- slirp/udp.c | 7 ++++--- slirp/udp.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-)
slirp/ip_icmp.c +1 -1
@@ -162,7 +162,7 @@ icmp_input(struct mbuf *m, int hlen)if (icmp_send (so, m, hlen) ==0 ) {return ; } -if (udp_attach (so) ==-1 ) { +if (udp_attach (so,AF_INET ) ==-1 ) {DEBUG_MISC ((dfd,"icmp_input udp_attach errno = %d-%s\n" , errno,strerror (errno)));sofree (so);
slirp/udp.c +4 -3
@@ -167,7 +167,7 @@ udp_input(register struct mbuf *m, int iphlen)if (!so) { goto bad; } -if (udp_attach (so) ==-1 ) { +if (udp_attach (so,AF_INET ) ==-1 ) {DEBUG_MISC ((dfd," udp_attach errno = %d-%s\n" , errno,strerror (errno)));sofree (so);
@@ -275,9 +275,10 @@ int udp_output(struct socket *so, struct mbuf *m, }int -udp_attach (struct socket *so) +udp_attach (struct socket *so,sa_family_t af) { -if ((so->s =qemu_socket (AF_INET ,SOCK_DGRAM ,0 )) !=-1 ) { + so->s =qemu_socket (af,SOCK_DGRAM ,0 ); +if (so->s !=-1 ) { so->so_expire = curtime +SO_EXPIRE ;insque (so, &so->slirp->udb); }
slirp/udp.h +1 -1
@@ -76,7 +76,7 @@ struct mbuf;void udp_init (Slirp *);void udp_cleanup (Slirp *);void udp_input (registerstruct mbuf *,int ); -int udp_attach (struct socket *); +int udp_attach (struct socket *,sa_family_t af);void udp_detach (struct socket *);struct socket *udp_listen (Slirp *,uint32_t ,u_int ,uint32_t ,u_int ,int );
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 07/18] slirp: Make udp_attach IPv6 compatible
Quoted 57 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
A
sa_family_tis now passed in argument toudp_attachinstead of using a hardcoded "AF_INET" to callqemu_socket().Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/ip_icmp.c | 2 +- slirp/udp.c | 7 ++++--- slirp/udp.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-)slirp/ip_icmp.c +1
-1@@ -162,7 +162,7 @@ icmp_input(struct mbuf *m, int hlen)if (icmp_send (so, m, hlen) ==0 ) {return ; } -if (udp_attach (so) ==-1 ) { +if (udp_attach (so,AF_INET ) ==-1 ) {DEBUG_MISC ((dfd,"icmp_input udp_attach errno = %d-%s\n" , errno,strerror (errno)));sofree (so);slirp/udp.c +4
-3@@ -167,7 +167,7 @@ udp_input(register struct mbuf *m, int iphlen)if (!so) { goto bad; } -if (udp_attach (so) ==-1 ) { +if (udp_attach (so,AF_INET ) ==-1 ) {DEBUG_MISC ((dfd," udp_attach errno = %d-%s\n" , errno,strerror (errno)));sofree (so);@@ -275,9 +275,10 @@ int udp_output(struct socket *so, struct mbuf *m, }int -udp_attach (struct socket *so) +udp_attach (struct socket *so,sa_family_t af) { -if ((so->s =qemu_socket (AF_INET ,SOCK_DGRAM ,0 )) !=-1 ) { + so->s =qemu_socket (af,SOCK_DGRAM ,0 ); +if (so->s !=-1 ) { so->so_expire = curtime +SO_EXPIRE ;insque (so, &so->slirp->udb); }slirp/udp.h +0
-0
slirp/udp.h struct mbuf; · line 82
Quoted 8 lines
slirp/udp.h +1
-1@@ -76,7 +76,7 @@ struct mbuf;void udp_init (Slirp *);void udp_cleanup (Slirp *);void udp_input (registerstruct mbuf *,int ); -int udp_attach (struct socket *); +int udp_attach (struct socket *,sa_family_t af);void udp_detach (struct socket *);struct socket *udp_listen (Slirp *,uint32_t ,u_int ,uint32_t ,u_int ,int );
PATCH 8/18 slirp: Adding family argument to tcp_fconnect()
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch simply adds a sa_family_t argument to remove the hardcoded "AF_INET" in the call of qemu_socket().
--- slirp/slirp.h | 2 +- slirp/tcp_input.c | 2 +- slirp/tcp_subr.c | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-)
slirp/slirp.h +1 -1
@@ -332,7 +332,7 @@ void tcp_respond(struct tcpcb *, register struct tcpiphdr *, register struct mbustruct tcpcb *tcp_newtcpcb (struct socket *);struct tcpcb *tcp_close (registerstruct tcpcb *);void tcp_sockclosed (struct tcpcb *); -int tcp_fconnect (struct socket *); +int tcp_fconnect (struct socket *,sa_family_t af);void tcp_connect (struct socket *);int tcp_attach (struct socket *);uint8_t tcp_tos (struct socket *);
slirp/tcp_input.c +1 -1
@@ -581,7 +581,7 @@ findso: goto cont_input; } -if ((tcp_fconnect (so) ==-1 ) && +if ((tcp_fconnect (so, so->so_ffamily) ==-1 ) && #if defined(_WIN32)socket_error () !=WSAEWOULDBLOCK #else
slirp/tcp_subr.c +3 -2
@@ -324,14 +324,15 @@ tcp_sockclosed(struct tcpcb *tp)* nonblocking. Connect returns after the SYN is sent, and does * not wait for ACK+SYN. */ -int tcp_fconnect (struct socket *so) +int tcp_fconnect (struct socket *so,sa_family_t af) {int ret=0 ;DEBUG_CALL ("tcp_fconnect" );DEBUG_ARG ("so = %p" , so); -if ( (ret = so->s =qemu_socket (AF_INET ,SOCK_STREAM ,0 )) >=0 ) { + ret = so->s =qemu_socket (af,SOCK_STREAM ,0 ); +if (ret >=0 ) {int opt, s=so->s;struct sockaddr_storage addr;
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 08/18] slirp: Adding family argument to tcp_fconnect()
Quoted 51 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
This patch simply adds a
sa_family_targument to remove the hardcoded "AF_INET" in the call ofqemu_socket().Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/slirp.h | 2 +- slirp/tcp_input.c | 2 +- slirp/tcp_subr.c | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-)slirp/slirp.h +1
-1@@ -332,7 +332,7 @@ void tcp_respond(struct tcpcb *, register struct tcpiphdr *, register struct mbustruct tcpcb *tcp_newtcpcb (struct socket *);struct tcpcb *tcp_close (registerstruct tcpcb *);void tcp_sockclosed (struct tcpcb *); -int tcp_fconnect (struct socket *); +int tcp_fconnect (struct socket *,sa_family_t af);void tcp_connect (struct socket *);int tcp_attach (struct socket *);uint8_t tcp_tos (struct socket *);slirp/tcp_input.c +1
-1@@ -581,7 +581,7 @@ findso: goto cont_input; } -if ((tcp_fconnect (so) ==-1 ) && +if ((tcp_fconnect (so, so->so_ffamily) ==-1 ) && #if defined(_WIN32)socket_error () !=WSAEWOULDBLOCK #elseslirp/tcp_subr.c +1
-1@@ -324,14 +324,15 @@ tcp_sockclosed(struct tcpcb *tp)* nonblocking. Connect returns after the SYN is sent, and does * not wait for ACK+SYN. */ -int tcp_fconnect (struct socket *so) +int tcp_fconnect (struct socket *so,sa_family_t af) {int ret=0 ;
slirp/tcp_subr.c tcp_sockclosed(struct tcpcb *tp) · line 338
Quoted 10 lines
slirp/tcp_subr.c +2
-1DEBUG_CALL ("tcp_fconnect" );DEBUG_ARG ("so = %p" , so); -if ( (ret = so->s =qemu_socket (AF_INET ,SOCK_STREAM ,0 )) >=0 ) { + ret = so->s =qemu_socket (af,SOCK_STREAM ,0 ); +if (ret >=0 ) {int opt, s=so->s;struct sockaddr_storage addr;
PATCH 9/18 qemu/timer.h : Adding function to second scale
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch adds SCALE_S, timer_new_s(), and qemu_clock_get_s in qemu/timer.h to manage second-scale timers.
--- include/qemu/timer.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
include/qemu/timer.h +32 -0
@@ -10,6 +10,7 @@/* timers */ +#define SCALE_S 1000000000#define SCALE_MS 1000000#define SCALE_US 1000#define SCALE_NS 1
@@ -92,6 +93,20 @@ extern QEMUTimerListGroup main_loop_tlg;int64_t qemu_clock_get_ns (QEMUClockType type);/** +* qemu_clock_get_s; +* @type: the clock type +* +* Get the second value of a clock with +* type @type +* +* Returns: the clock value in seconds +*/ +static inline int64_t qemu_clock_get_s (QEMUClockType type) +{ +return qemu_clock_get_ns (type) /SCALE_S ; +} + +/** * qemu_clock_get_ms; * @type: the clock type *
@@ -598,6 +613,23 @@ static inline QEMUTimer *timer_new_ms(QEMUClockType type, QEMUTimerCB *cb, }/** +* timer_new_s: +* @clock: the clock to associate with the timer +* @callback: the callback to call when the timer expires +* @opaque: the opaque pointer to pass to the callback +* +* Create a new timer with second scale on the default timer list +* associated with the clock. +* +* Returns: a pointer to the newly created timer +*/ +static inline QEMUTimer *timer_new_s (QEMUClockType type,QEMUTimerCB *cb, +void *opaque) +{ +return timer_new (type,SCALE_S , cb, opaque); +} + +/** * timer_deinit: * @ts: the timer to be de-initialised *
Signature · 1 line
2.6.2
PATCH 10/18 slirp: Adding IPv6, ICMPv6 Echo and NDP autoconfiguration
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch adds the functions needed to handle IPv6 packets. ICMPv6 and NDP headers are implemented.
Slirp is now able to send NDP Router or Neighbor Advertisement when it receives Router or Neighbor Solicitation. Using a 64bit-sized IPv6 prefix, the guest is now able to perform stateless autoconfiguration (SLAAC) and to compute its IPv6 address.
This patch adds an ndp_table, mainly inspired by arp_table, to keep an NDP cache and manage network address resolution.
Slirp regularly sends NDP Neighbor Advertisement, as recommended by the RFC, to make the guest refresh its route.
This also adds ip6_cksum() to compute ICMPv6 checksums using IPv6 pseudo-header.
Some #define ETH_* are moved upper in slirp.h to make them accessible to other slirp/*.h
--- slirp/Makefile.objs | 4 +- slirp/cksum.c | 23 ++++ slirp/if.c | 2 +- slirp/ip6.h | 139 +++++++++++++++++++++ slirp/ip6_icmp.c | 350 ++++++++++++++++++++++++++++++++++++++++++++++++++++ slirp/ip6_icmp.h | 248 +++++++++++++++++++++++++++++++++++++ slirp/ip6_input.c | 75 +++++++++++ slirp/ip6_output.c | 41 ++++++ slirp/ndp_table.c | 87 +++++++++++++ slirp/slirp.c | 47 +++++-- slirp/slirp.h | 47 ++++++- slirp/socket.h | 7 ++ 12 files changed, 1053 insertions(+), 17 deletions(-) create mode 100644 slirp/ip6.h create mode 100644 slirp/ip6_icmp.c create mode 100644 slirp/ip6_icmp.h create mode 100644 slirp/ip6_input.c create mode 100644 slirp/ip6_output.c create mode 100644 slirp/ndp_table.c
slirp/Makefile.objs +2 -2
@@ -1,3 +1,3 @@ -common-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o dnssearch.o +common-obj-y = cksum.o if.o ip_icmp.o ip6_icmp.o ip6_input.o ip6_output.o ip_input.o ip_output.o dnssearch.o common-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o -common-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o +common-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o ndp_table.o
slirp/cksum.c +23 -0
@@ -137,3 +137,26 @@ cont:REDUCE ;return (~sum &0xffff ); } + +int ip6_cksum (struct mbuf *m) +{ + struct ip6 save_ip, *ip = mtod(m, struct ip6 *); + struct ip6_pseudohdr *ih = mtod(m, struct ip6_pseudohdr *); +int sum; + + save_ip = *ip; + + ih->ih_src = save_ip.ip_src; + ih->ih_dst = save_ip.ip_dst; + ih->ih_pl =htonl ((uint32_t )ntohs (save_ip.ip_pl)); + ih->ih_zero_hi =0 ; + ih->ih_zero_lo =0 ; + ih->ih_nh = save_ip.ip_nh; + + sum =cksum (m, ((int )sizeof (struct ip6_pseudohdr )) + +ntohl (ih->ih_pl)); + + *ip = save_ip; + +return sum; +}
slirp/if.c +1 -1
@@ -193,7 +193,7 @@ void if_start(Slirp *slirp)/* Try to send packet unless it already expired */ if (ifm->expiration_date >= now && !if_encap (slirp, ifm)) { -/* Packet is delayed due to pending ARP resolution */ +/* Packet is delayed due to pending ARP or NDP resolution */ continue ; }
slirp/ip6.h new file +139 -0
139 lines not on this page · raw message
slirp/ip6_icmp.c new file +350 -0
350 lines not on this page · raw message
slirp/ip6_icmp.h new file +248 -0
248 lines not on this page · raw message
slirp/ip6_input.c new file +75 -0
75 lines not on this page · raw message
slirp/ip6_output.c new file +41 -0
41 lines not on this page · raw message
slirp/ndp_table.c new file +87 -0
87 lines not on this page · raw message
slirp/slirp.c +39 -8
95 lines not on this page · raw message
slirp/slirp.h +41 -6
97 lines not on this page · raw message
slirp/socket.h +7 -0
@@ -101,6 +101,13 @@ static inline int sockaddr_equal(struct sockaddr_storage *a,return (a4->sin_addr.s_addr == b4->sin_addr.s_addr && a4->sin_port == b4->sin_port); } +case AF_INET6 : + { +struct sockaddr_in6 *a6 = (struct sockaddr_in6 *) a; +struct sockaddr_in6 *b6 = (struct sockaddr_in6 *) b; +return (in6_equal (a6->sin6_addr, b6->sin6_addr) + && a6->sin6_port == b6->sin6_port); + }default :assert (0 ); }
Signature · 1 line
2.6.2
PATCH 11/18 slirp: Adding ICMPv6 error sending
In reply to Samuel ThibaultFrom: Yann Bordenave <[email protected]>
Disambiguation : icmp_error is renamed into icmp_send_error, since it doesn't manage errors, but only sends ICMP Error messages.
Adding icmp6_send_error to send ICMPv6 Error messages. This function is simpler than the v4 version.
Adding some calls in various functions to send ICMP errors, when a received packet is too big, or when its hop limit is 0.
--- slirp/ip6_icmp.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- slirp/ip6_icmp.h | 10 +++++++++ slirp/ip6_input.c | 16 ++++++++------ slirp/ip_icmp.c | 12 +++++----- slirp/ip_icmp.h | 4 ++-- slirp/ip_input.c | 8 +++---- slirp/socket.c | 4 ++-- slirp/tcp_input.c | 2 +- slirp/udp.c | 3 ++- 9 files changed, 100 insertions(+), 25 deletions(-)
slirp/ip6_icmp.c +64 -2
@@ -20,7 +20,8 @@static void ra_timer_handler (void *opaque) {Slirp *slirp = opaque; -timer_mod (slirp->ra_timer,qemu_clock_get_s (QEMU_CLOCK_VIRTUAL ) + NDP_Interval); +timer_mod (slirp->ra_timer, +qemu_clock_get_s (QEMU_CLOCK_VIRTUAL ) + NDP_Interval);ndp_send_ra (slirp); }
@@ -28,7 +29,8 @@ void icmp6_init(Slirp *slirp) {srand (time (NULL )); slirp->ra_timer =timer_new_s (QEMU_CLOCK_VIRTUAL , ra_timer_handler, slirp); -timer_mod (slirp->ra_timer,qemu_clock_get_s (QEMU_CLOCK_VIRTUAL ) + NDP_Interval); +timer_mod (slirp->ra_timer, +qemu_clock_get_s (QEMU_CLOCK_VIRTUAL ) + NDP_Interval); }#undef NDP_Interval
@@ -65,6 +67,66 @@ static void icmp6_send_echoreply(struct mbuf *m, Slirp *slirp, struct ip6 *ip,ip6_output (NULL , t,0 ); } +void icmp6_send_error (struct mbuf *m,uint8_t type,uint8_t code) +{ +Slirp *slirp = m->slirp; +struct mbuf *t =m_get (slirp); + struct ip6 *ip = mtod(m, struct ip6 *); + +char addrstr[INET6_ADDRSTRLEN ]; +DEBUG_CALL ("icmp_send_error" ); +DEBUG_ARGS ((dfd," type = %d, code = %d\n" , type, code)); + +/* IPv6 packet */ + struct ip6 *rip = mtod(t, struct ip6 *); + rip->ip_src = (struct in6_addr )LINKLOCAL_ADDR ; +if (in6_multicast (ip->ip_src) ||in6_unspecified (ip->ip_src)) { +/* :TODO:maethor:130317: icmp error? */ +return ; + } + rip->ip_dst = ip->ip_src; +inet_ntop (AF_INET6 , &rip->ip_dst, addrstr,INET6_ADDRSTRLEN ); +DEBUG_ARG ("target = %s" , addrstr); + + rip->ip_nh =IPPROTO_ICMPV6 ; +const int error_data_len =min (m->m_len, +IF_MTU - (sizeof (struct ip6 ) +ICMP6_ERROR_MINLEN )); + rip->ip_pl =htons (ICMP6_ERROR_MINLEN + error_data_len); + t->m_len =sizeof (struct ip6 ) +ntohs (rip->ip_pl); + +/* ICMPv6 packet */ + t->m_data +=sizeof (struct ip6 ); + struct icmp6 *ricmp = mtod(t, struct icmp6 *); + ricmp->icmp6_type = type; + ricmp->icmp6_code = code; + ricmp->icmp6_cksum =0 ; + +switch (type) { +case ICMP6_UNREACH : +case ICMP6_TIMXCEED : + ricmp->icmp6_err.unused =0 ; +break ; +case ICMP6_TOOBIG : + ricmp->icmp6_err.mtu =htonl (IF_MTU ); +break ; +case ICMP6_PARAMPROB : +/* :TODO:Meow:130316: Handle this case */ +break ; +default : +assert (0 ); +break ; + } + t->m_data +=ICMP6_ERROR_MINLEN ; +memcpy (t->m_data, m->m_data, error_data_len); + +/* Checksum */ + t->m_data -=ICMP6_ERROR_MINLEN ; + t->m_data -=sizeof (struct ip6 ); + ricmp->icmp6_cksum =ip6_cksum (t); + +ip6_output (NULL , t,0 ); +} +/* * Process a NDP message */
slirp/ip6_icmp.h +10 -0
@@ -22,6 +22,12 @@ struct icmp6_echo { /* Echo Messages */uint16_t seq_num; }; +union icmp6_error_body { +uint32_t unused; +uint32_t pointer; +uint32_t mtu; +}; +/* * NDP Messages */
@@ -85,6 +91,7 @@ struct icmp6 {uint8_t icmp6_code;/* type sub code */ uint16_t icmp6_cksum;/* ones complement cksum of struct */ union { +union icmp6_error_body error_body;struct icmp6_echo echo;struct ndp_rs ndp_rs;struct ndp_ra ndp_ra;
@@ -92,6 +99,7 @@ struct icmp6 {struct ndp_na ndp_na;struct ndp_redirect ndp_redirect; } icmp6_body; +#define icmp6_err icmp6_body.error_body#define icmp6_echo icmp6_body.echo#define icmp6_nrs icmp6_body.ndp_rs#define icmp6_nra icmp6_body.ndp_ra
@@ -101,6 +109,7 @@ struct icmp6 { }QEMU_PACKED ;#define ICMP6_MINLEN 4 +#define ICMP6_ERROR_MINLEN 8#define ICMP6_ECHO_MINLEN 8#define ICMP6_NDP_RS_MINLEN 8#define ICMP6_NDP_RA_MINLEN 16
@@ -242,6 +251,7 @@ void icmp6_input(struct mbuf *);void icmp6_error(struct mbuf *msrc, u_char type, u_char code, int minsize, const char *message); */ +void icmp6_send_error (struct mbuf *m,uint8_t type,uint8_t code);void ndp_send_ra (Slirp *slirp);void ndp_send_ns (Slirp *slirp,struct in6_addr addr);
slirp/ip6_input.c +9 -7
@@ -33,7 +33,7 @@ void ip6_input(struct mbuf *m)DEBUG_ARG ("m_len = %d" , m->m_len);if (m->m_len <sizeof (struct ip6 )) { -return ; + goto bad; } ip6 = mtod(m, struct ip6 *);
@@ -42,10 +42,14 @@ void ip6_input(struct mbuf *m) goto bad; } +if (ntohs (ip6->ip_pl) >IF_MTU ) { +icmp6_send_error (m,ICMP6_TOOBIG ,0 ); + goto bad; + } +/* check ip_ttl for a correct ICMP reply */ if (ip6->ip_hl ==0 ) { -/* :TODO:maethor:130307: icmp6_error */ -/*icmp_error(m, ICMP_TIMXCEED,ICMP_TIMXCEED_INTRANS, 0,"ttl");*/ +icmp6_send_error (m,ICMP6_TIMXCEED ,ICMP6_TIMXCEED_INTRANS ); goto bad; }
@@ -53,16 +57,14 @@ void ip6_input(struct mbuf *m)* Switch out to protocol's input routine. */ switch (ip6->ip_nh) { -#if 0 case IPPROTO_TCP :/* :TODO:maethor:130307: TCP */ -tcp_input (m, hlen, (struct socket *)NULL ); +icmp6_send_error (m,ICMP6_UNREACH ,ICMP6_UNREACH_NO_ROUTE );break ;case IPPROTO_UDP :/* :TODO:maethor:130312: UDP */ -udp_input (m, hlen); +icmp6_send_error (m,ICMP6_UNREACH ,ICMP6_UNREACH_NO_ROUTE );break ; -#endif case IPPROTO_ICMPV6 :icmp6_input (m);break ;
slirp/ip_icmp.c +6 -6
@@ -37,7 +37,7 @@/* Be nice and tell them it's just a pseudo-ping packet */ static const char icmp_ping_msg[] ="This is a pseudo-PING packet used by Slirp to emulate ICMP ECHO-REQUEST packets.\n" ; -/* list of actions for icmp_error() on RX of an icmp message */ +/* list of actions for icmp_send_error() on RX of an icmp message */ static const int icmp_flush[19 ] = {/* ECHO REPLY (0) */ 0 ,1 ,
@@ -100,7 +100,7 @@ static int icmp_send(struct socket *so, struct mbuf *m, int hlen) (struct sockaddr *)&addr, sizeof(addr)) == -1) {DEBUG_MISC ((dfd,"icmp_input icmp sendto tx errno = %d-%s\n" , errno,strerror (errno))); -icmp_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno)); +icmp_send_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno));icmp_detach (so); }
@@ -188,7 +188,7 @@ icmp_input(struct mbuf *m, int hlen) (struct sockaddr *)&addr, sizeof(addr)) == -1) {DEBUG_MISC ((dfd,"icmp_input udp sendto tx errno = %d-%s\n" , errno,strerror (errno))); -icmp_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno)); +icmp_send_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno));udp_detach (so); } }/* if ip->ip_dst.s_addr == alias_addr.s_addr */
@@ -234,7 +234,7 @@ end_error:#define ICMP_MAXDATALEN (IP_MSS-28)void -icmp_error (struct mbuf *msrc,u_char type,u_char code,int minsize, +icmp_send_error (struct mbuf *msrc,u_char type,u_char code,int minsize,const char *message) {unsigned hlen, shlen, s_ip_len;
@@ -242,7 +242,7 @@ icmp_error(struct mbuf *msrc, u_char type, u_char code, int minsize, registerstruct icmp *icp; registerstruct mbuf *m; -DEBUG_CALL ("icmp_error" ); +DEBUG_CALL ("icmp_send_error" );DEBUG_ARG ("msrc = %p" , msrc);DEBUG_ARG ("msrc_len = %d" , msrc->m_len);
@@ -432,7 +432,7 @@ void icmp_receive(struct socket *so) }DEBUG_MISC ((dfd," udp icmp rx errno = %d-%s\n" , errno,strerror (errno))); -icmp_error (so->so_m,ICMP_UNREACH , error_code,0 ,strerror (errno)); +icmp_send_error (so->so_m,ICMP_UNREACH , error_code,0 ,strerror (errno)); }else {icmp_reflect (so->so_m); so->so_m =NULL ;/* Don't m_free() it again! */
slirp/ip_icmp.h +2 -2
@@ -156,8 +156,8 @@ struct icmp {void icmp_init (Slirp *slirp);void icmp_cleanup (Slirp *slirp);void icmp_input (struct mbuf *,int ); -void icmp_error (struct mbuf *msrc,u_char type,u_char code,int minsize, -const char *message); +void icmp_send_error (struct mbuf *msrc,u_char type,u_char code,int minsize, +const char *message);void icmp_reflect (struct mbuf *);void icmp_receive (struct socket *so);void icmp_detach (struct socket *so);
slirp/ip_input.c +4 -4
@@ -131,9 +131,9 @@ ip_input(struct mbuf *m)m_adj (m, ip->ip_len - m->m_len);/* check ip_ttl for a correct ICMP reply */ -if (ip->ip_ttl==0 ) { -icmp_error (m,ICMP_TIMXCEED ,ICMP_TIMXCEED_INTRANS ,0 ,"ttl" ); - goto bad; +if (ip->ip_ttl ==0 ) { +icmp_send_error (m,ICMP_TIMXCEED ,ICMP_TIMXCEED_INTRANS ,0 ,"ttl" ); + goto bad; }/*
@@ -636,7 +636,7 @@ typedef uint32_t n_time; }return (0 ); bad: -icmp_error (m, type, code,0 ,0 ); +icmp_send_error (m, type, code,0 ,0 );return (1 ); }
slirp/socket.c +2 -2
@@ -462,7 +462,7 @@ sorecvfrom(struct socket *so)DEBUG_MISC ((dfd," udp icmp rx errno = %d-%s\n" , errno,strerror (errno))); -icmp_error (so->so_m,ICMP_UNREACH ,code,0 ,strerror (errno)); +icmp_send_error (so->so_m,ICMP_UNREACH , code,0 ,strerror (errno)); }else {icmp_reflect (so->so_m); so->so_m =NULL ;/* Don't m_free() it again! */
@@ -510,7 +510,7 @@ sorecvfrom(struct socket *so)else if (errno ==ENETUNREACH ) code=ICMP_UNREACH_NET ;DEBUG_MISC ((dfd," rx error, tx icmp ICMP_UNREACH:%i\n" , code)); -icmp_error (so->so_m,ICMP_UNREACH ,code,0 ,strerror (errno)); +icmp_send_error (so->so_m,ICMP_UNREACH , code,0 ,strerror (errno));m_free (m); }else {/*
slirp/tcp_input.c +1 -1
@@ -604,7 +604,7 @@ findso: m->m_data -=sizeof (struct tcpiphdr )+off-sizeof (struct tcphdr ); m->m_len +=sizeof (struct tcpiphdr )+off-sizeof (struct tcphdr ); *ip=save_ip; -icmp_error (m,ICMP_UNREACH ,code,0 ,strerror (errno)); +icmp_send_error (m,ICMP_UNREACH , code,0 ,strerror (errno)); }tcp_close (tp);m_free (m);
slirp/udp.c +2 -1
@@ -206,7 +206,8 @@ udp_input(register struct mbuf *m, int iphlen) m->m_data -= iphlen; *ip=save_ip;DEBUG_MISC ((dfd,"udp tx errno = %d-%s\n" ,errno,strerror (errno))); -icmp_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno)); +icmp_send_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 , +strerror (errno)); goto bad; }
Signature · 1 line
2.6.2
PATCH 12/18 slirp: Adding IPv6 UDP support
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This adds the sin6 case in the fhost and lhost unions and related macros. It adds udp6_input() and udp6_output().
It adds the IPv6 case in sorecvfrom().
Finally, udp_input() is called by ip6_input().
--- slirp/Makefile.objs | 2 +- slirp/ip6_input.c | 3 +- slirp/socket.c | 7 ++- slirp/socket.h | 6 +++ slirp/udp.h | 5 ++ slirp/udp6.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 slirp/udp6.c
slirp/Makefile.objs +1 -1
@@ -1,3 +1,3 @@ common-obj-y = cksum.o if.o ip_icmp.o ip6_icmp.o ip6_input.o ip6_output.o ip_input.o ip_output.o dnssearch.o common-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o -common-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o ndp_table.o +common-obj-y += tcp_subr.o tcp_timer.o udp.o udp6.o bootp.o tftp.o arp_table.o ndp_table.o
slirp/ip6_input.c +1 -2
@@ -62,8 +62,7 @@ void ip6_input(struct mbuf *m)icmp6_send_error (m,ICMP6_UNREACH ,ICMP6_UNREACH_NO_ROUTE );break ;case IPPROTO_UDP : -/* :TODO:maethor:130312: UDP */ -icmp6_send_error (m,ICMP6_UNREACH ,ICMP6_UNREACH_NO_ROUTE ); +udp6_input (m);break ;case IPPROTO_ICMPV6 :icmp6_input (m);
slirp/socket.c +6 -1
@@ -540,8 +540,13 @@ sorecvfrom(struct socket *so) (struct sockaddr_in *) &daddr, so->so_iptos);break ; -default : +case AF_INET6 : +udp6_output (so, m, (struct sockaddr_in6 *) &saddr, + (struct sockaddr_in6 *) &daddr);break ; +default : +assert (0 ); +break ; } }/* rx error */ }/* if ping packet */
slirp/socket.h +6 -0
@@ -34,17 +34,23 @@ struct socket {union {/* foreign host */ struct sockaddr_storage ss;struct sockaddr_in sin; +struct sockaddr_in6 sin6; } fhost;#define so_faddr fhost.sin.sin_addr#define so_fport fhost.sin.sin_port +#define so_faddr6 fhost.sin6.sin6_addr +#define so_fport6 fhost.sin6.sin6_port#define so_ffamily fhost.ss.ss_familyunion {/* local host */ struct sockaddr_storage ss;struct sockaddr_in sin; +struct sockaddr_in6 sin6; } lhost;#define so_laddr lhost.sin.sin_addr#define so_lport lhost.sin.sin_port +#define so_laddr6 lhost.sin6.sin6_addr +#define so_lport6 lhost.sin6.sin6_port#define so_lfamily lhost.ss.ss_familyuint8_t so_iptos;/* Type of service */
slirp/udp.h +5 -0
@@ -83,4 +83,9 @@ struct socket * udp_listen(Slirp *, uint32_t, u_int, uint32_t, u_int,int udp_output (struct socket *so,struct mbuf *m,struct sockaddr_in *saddr,struct sockaddr_in *daddr,int iptos); + +void udp6_input (registerstruct mbuf *); +int udp6_output (struct socket *so,struct mbuf *m, +struct sockaddr_in6 *saddr,struct sockaddr_in6 *daddr); +#endif
slirp/udp6.c new file +149 -0
149 lines not on this page · raw message
Signature · 1 line
2.6.2
PATCH 13/18 slirp: Factorizing tcpiphdr structure with an union
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch factorizes the tcpiphdr structure to put the IPv4 fields in an union, for addition of version 6 in further patch.
Using some macros, retrocompatibility of the existing code is assured.
This patch also fixes the SLIRP_MSIZE and margin computation in various functions, and makes them compatible with the new tcpiphdr structure, whose size will be bigger than sizeof(struct tcphdr) + sizeof(struct ip)
--- slirp/if.h | 4 ++-- slirp/mbuf.c | 3 ++- slirp/slirp.c | 15 ++++++++------- slirp/socket.c | 13 ++++++++++++- slirp/tcp_input.c | 31 ++++++++++++++++++++----------- slirp/tcp_output.c | 18 +++++++++++++----- slirp/tcp_subr.c | 31 ++++++++++++++++++++++--------- slirp/tcpip.h | 31 +++++++++++++++++++++++-------- 8 files changed, 102 insertions(+), 44 deletions(-)
slirp/if.h +2 -2
@@ -17,7 +17,7 @@#define IF_MRU 1500#define IF_COMP IF_AUTOCOMP/* Flags for compression */ -/* 2 for alignment, 14 for ethernet, 40 for TCP/IP */ -#define IF_MAXLINKHDR (2 + 14 + 40) +/* 2 for alignment, 14 for ethernet */ +#define IF_MAXLINKHDR (2 + ETH_HLEN)#endif
slirp/mbuf.c +2 -1
@@ -23,7 +23,8 @@* Find a nice value for msize * XXX if_maxlinkhdr already in mtu */ -#define SLIRP_MSIZE (IF_MTU + IF_MAXLINKHDR + offsetof(struct mbuf, m_dat) + 6) +#define SLIRP_MSIZE \ + (offsetof(struct mbuf, m_dat) + IF_MAXLINKHDR + TCPIPHDR_DELTA + IF_MTU)void m_init (Slirp *slirp)
slirp/slirp.c +8 -7
@@ -754,15 +754,16 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) m =m_get (slirp);if (!m)return ; -/* Note: we add to align the IP header */ -if (M_FREEROOM (m) < pkt_len +2 ) { -m_inc (m, pkt_len +2 ); +/* Note: we add 2 to align the IP header on 4 bytes, +* and add the margin for the tcpiphdr overhead */ +if (M_FREEROOM (m) < pkt_len +TCPIPHDR_DELTA +2 ) { +m_inc (m, pkt_len +TCPIPHDR_DELTA +2 ); } - m->m_len = pkt_len +2 ; -memcpy (m->m_data +2 , pkt, pkt_len); + m->m_len = pkt_len +TCPIPHDR_DELTA +2 ; +memcpy (m->m_data +TCPIPHDR_DELTA +2 , pkt, pkt_len); - m->m_data +=2 +ETH_HLEN ; - m->m_len -=2 +ETH_HLEN ; + m->m_data +=TCPIPHDR_DELTA +2 +ETH_HLEN ; + m->m_len -=TCPIPHDR_DELTA +2 +ETH_HLEN ;if (proto ==ETH_P_IP ) {ip_input (m);
slirp/socket.c +12 -1
@@ -482,7 +482,18 @@ sorecvfrom(struct socket *so)if (!m) {return ; } - m->m_data +=IF_MAXLINKHDR ; +switch (so->so_ffamily) { +case AF_INET : + m->m_data +=IF_MAXLINKHDR +sizeof (struct udpiphdr ); +break ; +case AF_INET6 : + m->m_data +=IF_MAXLINKHDR +sizeof (struct ip6 ) + +sizeof (struct udphdr ); +break ; +default : +assert (0 ); +break ; + }/* * XXX Shouldn't FIONREAD packets destined for port 53,
slirp/tcp_input.c +20 -11
@@ -254,11 +254,6 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso) } slirp = m->slirp; -/* -* Get IP and TCP header together in first mbuf. -* Note: IP leaves IP header in first mbuf. -*/ - ti = mtod(m, struct tcpiphdr *);if (iphlen >sizeof (struct ip )) {ip_stripoptions (m, (struct mbuf *)0 ); iphlen=sizeof (struct ip );
@@ -275,14 +270,28 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso) save_ip.ip_len+= iphlen;/* +* Get IP and TCP header together in first mbuf. +* Note: IP leaves IP header in first mbuf. +*/ + m->m_data -=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) + +sizeof (struct tcphdr )); + m->m_len +=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) + +sizeof (struct tcphdr )); + ti = mtod(m, struct tcpiphdr *); + +/* * Checksum extended TCP header and data. */ - tlen = ((struct ip *)ti)->ip_len; -tcpiphdr2qlink (ti)->next =tcpiphdr2qlink (ti)->prev =NULL ; -memset (&ti->ti_i.ih_mbuf,0 ,sizeof (struct mbuf_ptr )); - ti->ti_x1 =0 ; + tlen = ip->ip_len; +tcpiphdr2qlink (ti)->next =tcpiphdr2qlink (ti)->prev =NULL ; +memset (&ti->ih_mbuf,0 ,sizeof (struct mbuf_ptr )); +memset (&ti->ti,0 ,sizeof (ti->ti)); + ti->ti_x0 =0 ; + ti->ti_src = save_ip.ip_src; + ti->ti_dst = save_ip.ip_dst; + ti->ti_pr = save_ip.ip_p; ti->ti_len =htons ((uint16_t )tlen); - len =sizeof (struct ip ) + tlen; + len = ((sizeof (struct tcpiphdr ) -sizeof (struct tcphdr )) + tlen);if (cksum (m, len)) { goto drop; }
@@ -1471,7 +1480,7 @@ tcp_mss(struct tcpcb *tp, u_int offer)DEBUG_ARG ("tp = %p" , tp);DEBUG_ARG ("offer = %d" , offer); - mss =min (IF_MTU ,IF_MRU ) -sizeof (struct tcpiphdr ); + mss =min (IF_MTU ,IF_MRU ) -sizeof (struct tcphdr ) +sizeof (struct ip );if (offer) mss =min (mss, offer); mss =max (mss,32 );
slirp/tcp_output.c +13 -5
@@ -447,15 +447,23 @@ send:*/ m->m_len = hdrlen + len;/* XXX Needed? m_len should be correct */ - { + struct tcpiphdr tcpiph_save = *(mtod(m, struct tcpiphdr *)); + m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + struct ip *ip = mtod(m, struct ip *); - ((struct ip *)ti)->ip_len = m->m_len; + ip->ip_len = m->m_len; + ip->ip_dst = tcpiph_save.ti_dst; + ip->ip_src = tcpiph_save.ti_src; + ip->ip_p = tcpiph_save.ti_pr; - ((struct ip *)ti)->ip_ttl =IPDEFTTL ; - ((struct ip *)ti)->ip_tos = so->so_iptos; + ip->ip_ttl =IPDEFTTL ; + ip->ip_tos = so->so_iptos; error =ip_output (so, m); - } +if (error) { out:return (error);
slirp/tcp_subr.c +22 -9
@@ -75,9 +75,10 @@ tcp_template(struct tcpcb *tp) registerstruct tcpiphdr *n = &tp->t_template; n->ti_mbuf =NULL ; - n->ti_x1 =0 ; +memset (&n->ti,0 ,sizeof (n->ti)); + n->ti_x0 =0 ; n->ti_pr =IPPROTO_TCP ; - n->ti_len =htons (sizeof (struct tcpiphdr ) -sizeof (struct ip )); + n->ti_len =htons (sizeof (struct tcphdr )); n->ti_src = so->so_faddr; n->ti_dst = so->so_laddr; n->ti_sport = so->so_fport;
@@ -130,6 +131,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, m->m_data +=IF_MAXLINKHDR ; *mtod(m, struct tcpiphdr *) = *ti; ti = mtod(m, struct tcpiphdr *); +memset (&ti->ti,0 ,sizeof (ti->ti)); flags =TH_ACK ; }else {/*
@@ -149,8 +151,8 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, tlen +=sizeof (struct tcpiphdr ); m->m_len = tlen; - ti->ti_mbuf =NULL ; - ti->ti_x1 =0 ; + ti->ti_mbuf =NULL ; + ti->ti_x0 =0 ; ti->ti_seq =htonl (seq); ti->ti_ack =htonl (ack); ti->ti_x2 =0 ;
@@ -163,12 +165,23 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, ti->ti_urp =0 ; ti->ti_sum =0 ; ti->ti_sum =cksum (m, tlen); - ((struct ip *)ti)->ip_len = tlen; -if (flags &TH_RST ) - ((struct ip *)ti)->ip_ttl =MAXTTL ; -else - ((struct ip *)ti)->ip_ttl =IPDEFTTL ; + struct tcpiphdr tcpiph_save = *(mtod(m, struct tcpiphdr *)); + m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + struct ip *ip = mtod(m, struct ip *); + ip->ip_len = tlen; + ip->ip_dst = tcpiph_save.ti_dst; + ip->ip_src = tcpiph_save.ti_src; + ip->ip_p = tcpiph_save.ti_pr; + +if (flags &TH_RST ) { + ip->ip_ttl =MAXTTL ; + }else { + ip->ip_ttl =IPDEFTTL ; + } (void )ip_output ((struct socket *)0 , m); }
slirp/tcpip.h +23 -8
@@ -37,15 +37,23 @@* Tcp+ip header, after ip options removed. */ struct tcpiphdr { -struct ipovly ti_i;/* overlaid ip structure */ -struct tcphdr ti_t;/* tcp header */ +struct mbuf_ptr ih_mbuf;/* backpointer to mbuf */ +union { +struct { +struct in_addr ih_src;/* source internet address */ +struct in_addr ih_dst;/* destination internet address */ +uint8_t ih_x1;/* (unused) */ +uint8_t ih_pr;/* protocol */ + } ti_i4; + } ti; +uint16_t ti_x0; +uint16_t ti_len;/* protocol length */ +struct tcphdr ti_t;/* tcp header */ }; -#define ti_mbuf ti_i.ih_mbuf.mptr -#define ti_x1 ti_i.ih_x1 -#define ti_pr ti_i.ih_pr -#define ti_len ti_i.ih_len -#define ti_src ti_i.ih_src -#define ti_dst ti_i.ih_dst +#define ti_mbuf ih_mbuf.mptr +#define ti_pr ti.ti_i4.ih_pr +#define ti_src ti.ti_i4.ih_src +#define ti_dst ti.ti_i4.ih_dst#define ti_sport ti_t.th_sport#define ti_dport ti_t.th_dport#define ti_seq ti_t.th_seq
@@ -65,6 +73,13 @@ struct tcpiphdr {#define tcpfrag_list_end (F ,T ) (tcpiphdr2qlink(F) == (struct qlink*)(T))#define tcpfrag_list_empty (T ) ((T)->seg_next == (struct tcpiphdr*)(T)) +/* This is the difference between the size of a tcpiphdr structure, and the +* size of actual ip+tcp headers, rounded up since we need to align data. */ +#define TCPIPHDR_DELTA \ + (max(0,\ + (sizeof(struct tcpiphdr)\ + - sizeof(struct ip) - sizeof(struct tcphdr) + 3)&~3)) +/* * Just a clean way to get to the first byte * of the packet
Signature · 1 line
2.6.2
PATCH 14/18 slirp: Generalizing and neutralizing various TCP functions before adding IPv6 stuff
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
Basically, this patch adds some switch in various TCP functions to prepare them for the IPv6 case.
To have something to "switch" in tcp_input() and tcp_respond(), a new argument is used to give them the sa_family of the addresses they are working on.
--- slirp/ip_input.c | 2 +- slirp/slirp.c | 6 ++++-- slirp/slirp.h | 5 +++-- slirp/tcp_input.c | 53 +++++++++++++++++++++++++++++++++++++++++++++-------- slirp/tcp_output.c | 14 +++++++++++--- slirp/tcp_subr.c | 40 ++++++++++++++++++++++++++++++++-------- slirp/tcp_timer.c | 3 ++- 7 files changed, 98 insertions(+), 25 deletions(-)
slirp/ip_input.c +1 -1
@@ -199,7 +199,7 @@ ip_input(struct mbuf *m)*/ switch (ip->ip_p) {case IPPROTO_TCP : -tcp_input (m, hlen, (struct socket *)NULL ); +tcp_input (m, hlen, (struct socket *)NULL ,AF_INET );break ;case IPPROTO_UDP :udp_input (m, hlen);
slirp/slirp.c +4 -2
@@ -574,7 +574,8 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)/* * Continue tcp_input */ -tcp_input ((struct mbuf *)NULL ,sizeof (struct ip ), so); +tcp_input ((struct mbuf *)NULL ,sizeof (struct ip ), so, + so->so_ffamily);/* continue; */ }else { ret =sowrite (so);
@@ -623,7 +624,8 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error) } } -tcp_input ((struct mbuf *)NULL ,sizeof (struct ip ), so); +tcp_input ((struct mbuf *)NULL ,sizeof (struct ip ), so, + so->so_ffamily); }/* SS_ISFCONNECTING */ #endif }
slirp/slirp.h +3 -2
@@ -352,7 +352,7 @@ void ip6_input(struct mbuf *);int ip6_output (struct socket *,struct mbuf *,int fast);/* tcp_input.c */ -void tcp_input (registerstruct mbuf *,int ,struct socket *); +void tcp_input (registerstruct mbuf *,int ,struct socket *,sa_family_t af);int tcp_mss (registerstruct tcpcb *,u_int );/* tcp_output.c */
@@ -363,7 +363,8 @@ void tcp_setpersist(register struct tcpcb *);void tcp_init (Slirp *);void tcp_cleanup (Slirp *);void tcp_template (struct tcpcb *); -void tcp_respond (struct tcpcb *, registerstruct tcpiphdr *, registerstruct mbuf *,tcp_seq ,tcp_seq ,int ); +void tcp_respond (struct tcpcb *, registerstruct tcpiphdr *, + registerstruct mbuf *,tcp_seq ,tcp_seq ,int ,sa_family_t );struct tcpcb *tcp_newtcpcb (struct socket *);struct tcpcb *tcp_close (registerstruct tcpcb *);void tcp_sockclosed (struct tcpcb *);
slirp/tcp_input.c +45 -8
123 lines not on this page · raw message
slirp/tcp_output.c +11 -3
@@ -60,7 +60,8 @@ tcp_output(struct tcpcb *tp) registerlong len, win;int off, flags, error; registerstruct mbuf *m; - registerstruct tcpiphdr *ti; + registerstruct tcpiphdr *ti, tcpiph_save; +struct ip *ip;u_char opt[MAX_TCPOPTLEN ];unsigned optlen, hdrlen;int idle, sendalot;
@@ -446,13 +447,15 @@ send:* the template, but need a way to checksum without them. */ m->m_len = hdrlen + len;/* XXX Needed? m_len should be correct */ + tcpiph_save = *(mtod(m, struct tcpiphdr *)); - struct tcpiphdr tcpiph_save = *(mtod(m, struct tcpiphdr *)); +switch (so->so_ffamily) { +case AF_INET : m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) -sizeof (struct ip ); m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) -sizeof (struct ip ); - struct ip *ip = mtod(m, struct ip *); + ip = mtod(m, struct ip *); ip->ip_len = m->m_len; ip->ip_dst = tcpiph_save.ti_dst;
@@ -463,6 +466,11 @@ send: ip->ip_tos = so->so_iptos; error =ip_output (so, m); +break ; + +default : + goto out; + }if (error) { out:
slirp/tcp_subr.c +32 -8
@@ -77,12 +77,20 @@ tcp_template(struct tcpcb *tp) n->ti_mbuf =NULL ;memset (&n->ti,0 ,sizeof (n->ti)); n->ti_x0 =0 ; +switch (so->so_ffamily) { +case AF_INET : n->ti_pr =IPPROTO_TCP ; n->ti_len =htons (sizeof (struct tcphdr )); n->ti_src = so->so_faddr; n->ti_dst = so->so_laddr; n->ti_sport = so->so_fport; n->ti_dport = so->so_lport; +break ; + +default : +/* :TODO:maethor:130321: ? */ +break ; + } n->ti_seq =0 ; n->ti_ack =0 ;
@@ -109,7 +117,7 @@ tcp_template(struct tcpcb *tp)*/ void tcp_respond (struct tcpcb *tp,struct tcpiphdr *ti,struct mbuf *m, -tcp_seq ack,tcp_seq seq,int flags) +tcp_seq ack,tcp_seq seq,int flags,sa_family_t af) { registerint tlen;int win =0 ;
@@ -143,8 +151,15 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, m->m_len =sizeof (struct tcpiphdr ); tlen =0 ;#define xchg (a,b,type) { type t; t=a; a=b; b=t; } +switch (af) { +case AF_INET :xchg (ti->ti_dst.s_addr, ti->ti_src.s_addr, uint32_t);xchg (ti->ti_dport, ti->ti_sport, uint16_t); +break ; +default : +/* :TODO:maethor:130321: ? */ +break ; + }#undef xchg } ti->ti_len =htons ((u_short)(sizeof (struct tcphdr ) + tlen));
@@ -167,11 +182,15 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, ti->ti_sum =cksum (m, tlen); struct tcpiphdr tcpiph_save = *(mtod(m, struct tcpiphdr *)); +struct ip *ip; + +switch (af) { +case AF_INET : m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) -sizeof (struct ip ); m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) -sizeof (struct ip ); - struct ip *ip = mtod(m, struct ip *); + ip = mtod(m, struct ip *); ip->ip_len = tlen; ip->ip_dst = tcpiph_save.ti_dst; ip->ip_src = tcpiph_save.ti_src;
@@ -184,6 +203,12 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, } (void )ip_output ((struct socket *)0 , m); +break ; + +default : +/* :TODO:maethor:130321: ? */ +break ; + } }/*
@@ -387,8 +412,8 @@ void tcp_connect(struct socket *inso) {Slirp *slirp = inso->slirp;struct socket *so; -struct sockaddr_in addr; -socklen_t addrlen =sizeof (struct sockaddr_in ); +struct sockaddr_storage addr; +socklen_t addrlen =sizeof (struct sockaddr_storage );struct tcpcb *tp;int s, opt;
@@ -413,9 +438,8 @@ void tcp_connect(struct socket *inso)free (so);/* NOT sofree */ return ; } - so->so_lfamily =AF_INET ; - so->so_laddr = inso->so_laddr; - so->so_lport = inso->so_lport; + so->lhost = inso->lhost; + so->so_ffamily = inso->so_ffamily; }tcp_mss (sototcpcb (so),0 );
@@ -431,7 +455,7 @@ void tcp_connect(struct socket *inso)qemu_setsockopt (s,SOL_SOCKET ,SO_OOBINLINE , &opt,sizeof (int ));socket_set_nodelay (s); - so->fhost.sin = addr; + so->fhost.ss = addr;sotranslate_accept (so);/* Close the accept() socket, set right state */
slirp/tcp_timer.c +2 -1
@@ -277,7 +277,8 @@ tcp_timers(register struct tcpcb *tp, int timer)* correspondent TCP to respond. */ tcp_respond (tp, &tp->t_template, (struct mbuf *)NULL , - tp->rcv_nxt, tp->snd_una -1 ,0 ); + tp->rcv_nxt, tp->snd_una -1 ,0 , + tp->t_socket->so_ffamily); tp->t_timer[TCPT_KEEP ] =TCPTV_KEEPINTVL ; }else tp->t_timer[TCPT_KEEP ] =TCPTV_KEEP_IDLE ;
Signature · 1 line
2.6.2
PATCH 15/18 slirp: Reindent after refactoring
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
No code change.
--- slirp/tcp_input.c | 95 +++++++++++++++++++++++++++--------------------------- slirp/tcp_output.c | 29 ++++++++--------- slirp/tcp_subr.c | 50 ++++++++++++++-------------- 3 files changed, 87 insertions(+), 87 deletions(-)
slirp/tcp_input.c +48 -47
@@ -256,48 +256,48 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso, sa_family_t af)switch (af) {case AF_INET : -if (iphlen >sizeof (struct ip )) { -ip_stripoptions (m, (struct mbuf *)0 ); - iphlen=sizeof (struct ip ); - } -/* XXX Check if too short */ +if (iphlen >sizeof (struct ip )) { +ip_stripoptions (m, (struct mbuf *)0 ); + iphlen =sizeof (struct ip ); + } +/* XXX Check if too short */ -/* -* Save a copy of the IP header in case we want restore it -* for sending an ICMP error message in response. -*/ - ip=mtod(m, struct ip *); - save_ip = *ip; - save_ip.ip_len+= iphlen; +/* +* Save a copy of the IP header in case we want restore it +* for sending an ICMP error message in response. +*/ + ip = mtod(m, struct ip *); + save_ip = *ip; + save_ip.ip_len += iphlen; -/* -* Get IP and TCP header together in first mbuf. -* Note: IP leaves IP header in first mbuf. -*/ - m->m_data -=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) - +sizeof (struct tcphdr )); - m->m_len +=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) - +sizeof (struct tcphdr )); - ti = mtod(m, struct tcpiphdr *); +/* +* Get IP and TCP header together in first mbuf. +* Note: IP leaves IP header in first mbuf. +*/ + m->m_data -=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) + +sizeof (struct tcphdr )); + m->m_len +=sizeof (struct tcpiphdr ) - (sizeof (struct ip ) + +sizeof (struct tcphdr )); + ti = mtod(m, struct tcpiphdr *); -/* -* Checksum extended TCP header and data. -*/ - tlen = ip->ip_len; -tcpiphdr2qlink (ti)->next =tcpiphdr2qlink (ti)->prev =NULL ; -memset (&ti->ih_mbuf,0 ,sizeof (struct mbuf_ptr )); -memset (&ti->ti,0 ,sizeof (ti->ti)); - ti->ti_x0 =0 ; - ti->ti_src = save_ip.ip_src; - ti->ti_dst = save_ip.ip_dst; - ti->ti_pr = save_ip.ip_p; - ti->ti_len =htons ((uint16_t )tlen); - len = ((sizeof (struct tcpiphdr ) -sizeof (struct tcphdr )) + tlen); -if (cksum (m, len)) { - goto drop; - } -break ; +/* +* Checksum extended TCP header and data. +*/ + tlen = ip->ip_len; +tcpiphdr2qlink (ti)->next =tcpiphdr2qlink (ti)->prev =NULL ; +memset (&ti->ih_mbuf,0 ,sizeof (struct mbuf_ptr )); +memset (&ti->ti,0 ,sizeof (ti->ti)); + ti->ti_x0 =0 ; + ti->ti_src = save_ip.ip_src; + ti->ti_dst = save_ip.ip_dst; + ti->ti_pr = save_ip.ip_p; + ti->ti_len =htons ((uint16_t )tlen); + len = ((sizeof (struct tcpiphdr ) -sizeof (struct tcphdr )) + tlen); +if (cksum (m, len)) { + goto drop; + } +break ;default : goto drop;
@@ -341,10 +341,10 @@ findso: fhost.ss_family = af;switch (af) {case AF_INET : - ((struct sockaddr_in *)&lhost)->sin_addr = ti->ti_src; - ((struct sockaddr_in *)&lhost)->sin_port = ti->ti_sport; - ((struct sockaddr_in *)&fhost)->sin_addr = ti->ti_dst; - ((struct sockaddr_in *)&fhost)->sin_port = ti->ti_dport; + ((struct sockaddr_in *)&lhost)->sin_addr = ti->ti_src; + ((struct sockaddr_in *)&lhost)->sin_port = ti->ti_sport; + ((struct sockaddr_in *)&fhost)->sin_addr = ti->ti_dst; + ((struct sockaddr_in *)&fhost)->sin_port = ti->ti_dport;break ;default : goto drop;
@@ -402,7 +402,7 @@ findso:if (so->so_iptos ==0 ) {switch (af) {case AF_INET : - so->so_iptos = ((struct ip *)ti)->ip_tos; + so->so_iptos = ((struct ip *)ti)->ip_tos;break ;default : goto drop;
@@ -588,8 +588,8 @@ findso:* tcp_ctl once connected, otherwise connect */ if (af ==AF_INET && - (so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == - slirp->vnetwork_addr.s_addr) { + (so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) == + slirp->vnetwork_addr.s_addr) {if (so->so_faddr.s_addr != slirp->vhost_addr.s_addr && so->so_faddr.s_addr != slirp->vnameserver_addr.s_addr) {/* May be an add exec */
@@ -637,7 +637,7 @@ findso: m->m_len +=sizeof (struct tcpiphdr )+off-sizeof (struct tcphdr );switch (af) {case AF_INET : - *ip=save_ip; + *ip = save_ip;break ;default : goto drop;
@@ -1511,7 +1511,8 @@ tcp_mss(struct tcpcb *tp, u_int offer)switch (so->so_ffamily) {case AF_INET : - mss =min (IF_MTU ,IF_MRU ) -sizeof (struct tcphdr ) +sizeof (struct ip ); + mss =min (IF_MTU ,IF_MRU ) -sizeof (struct tcphdr ) + +sizeof (struct ip );break ;default :/* :TODO:maethor:130319: ??? */
slirp/tcp_output.c +14 -15
@@ -451,21 +451,20 @@ send:switch (so->so_ffamily) {case AF_INET : - m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) - -sizeof (struct ip ); - m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) - -sizeof (struct ip ); - ip = mtod(m, struct ip *); - - ip->ip_len = m->m_len; - ip->ip_dst = tcpiph_save.ti_dst; - ip->ip_src = tcpiph_save.ti_src; - ip->ip_p = tcpiph_save.ti_pr; - - ip->ip_ttl =IPDEFTTL ; - ip->ip_tos = so->so_iptos; - - error =ip_output (so, m); + m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip ); + ip = mtod(m, struct ip *); + + ip->ip_len = m->m_len; + ip->ip_dst = tcpiph_save.ti_dst; + ip->ip_src = tcpiph_save.ti_src; + ip->ip_p = tcpiph_save.ti_pr; + + ip->ip_ttl =IPDEFTTL ; + ip->ip_tos = so->so_iptos; + error =ip_output (so, m);break ;default :
slirp/tcp_subr.c +25 -25
68 lines not on this page · raw message
Signature · 1 line
2.6.2
PATCH 16/18 slirp: Handle IPv6 in TCP functions
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch adds IPv6 case in TCP functions refactored by the last patches.
This also adds IPv6 pseudo-header in tcpiphdr structure. Finally, tcp_input() is called by ip6_input().
--- slirp/ip6_input.c | 4 ++-- slirp/tcp.h | 2 ++ slirp/tcp_input.c | 58 +++++++++++++++++++++++++++++++++++++++++------------- slirp/tcp_output.c | 16 +++++++++++++++ slirp/tcp_subr.c | 36 +++++++++++++++++++++++++++++---- slirp/tcpip.h | 9 +++++++++ 6 files changed, 105 insertions(+), 20 deletions(-)
slirp/ip6_input.c +2 -2
@@ -58,8 +58,8 @@ void ip6_input(struct mbuf *m)*/ switch (ip6->ip_nh) {case IPPROTO_TCP : -/* :TODO:maethor:130307: TCP */ -icmp6_send_error (m,ICMP6_UNREACH ,ICMP6_UNREACH_NO_ROUTE ); +NTOHS (ip6->ip_pl); +tcp_input (m,sizeof (struct ip6 ), (struct socket *)NULL ,AF_INET6 );break ;case IPPROTO_UDP :udp6_input (m);
slirp/tcp.h +2 -0
@@ -106,6 +106,8 @@ struct tcphdr {*/ #undef TCP_MSS#define TCP_MSS 1460 +#undef TCP6_MSS +#define TCP6_MSS 1440#undef TCP_MAXWIN#define TCP_MAXWIN 65535/* largest value for (unscaled) window */
slirp/tcp_input.c +44 -14
113 lines not on this page · raw message
slirp/tcp_output.c +16 -0
@@ -62,6 +62,7 @@ tcp_output(struct tcpcb *tp) registerstruct mbuf *m; registerstruct tcpiphdr *ti, tcpiph_save;struct ip *ip; +struct ip6 *ip6;u_char opt[MAX_TCPOPTLEN ];unsigned optlen, hdrlen;int idle, sendalot;
@@ -467,6 +468,21 @@ send: error =ip_output (so, m);break ; +case AF_INET6 : + m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip6 ); + m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip6 ); + ip6 = mtod(m, struct ip6 *); + + ip6->ip_pl = tcpiph_save.ti_len; + ip6->ip_dst = tcpiph_save.ti_dst6; + ip6->ip_src = tcpiph_save.ti_src6; + ip6->ip_nh = tcpiph_save.ti_nh6; + + error =ip6_output (so, m,0 ); +break ; +default : goto out; }
slirp/tcp_subr.c +32 -4
@@ -87,8 +87,17 @@ tcp_template(struct tcpcb *tp) n->ti_dport = so->so_lport;break ; +case AF_INET6 : + n->ti_nh6 =IPPROTO_TCP ; + n->ti_len =htons (sizeof (struct tcphdr )); + n->ti_src6 = so->so_faddr6; + n->ti_dst6 = so->so_laddr6; + n->ti_sport = so->so_fport6; + n->ti_dport = so->so_lport6; +break ; +default : -/* :TODO:maethor:130321: ? */ +assert (0 );break ; }
@@ -156,8 +165,12 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m,xchg (ti->ti_dst.s_addr, ti->ti_src.s_addr, uint32_t);xchg (ti->ti_dport, ti->ti_sport, uint16_t);break ; +case AF_INET6 : + xchg(ti->ti_dst6, ti->ti_src6, struct in6_addr); +xchg (ti->ti_dport, ti->ti_sport, uint16_t); +break ;default : -/* :TODO:maethor:130321: ? */ +assert (0 );break ; }#undef xchg
@@ -183,6 +196,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, struct tcpiphdr tcpiph_save = *(mtod(m, struct tcpiphdr *));struct ip *ip; +struct ip6 *ip6;switch (af) {case AF_INET :
@@ -205,8 +219,22 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct mbuf *m, (void )ip_output ((struct socket *)0 , m);break ; +case AF_INET6 : + m->m_data +=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip6 ); + m->m_len -=sizeof (struct tcpiphdr ) -sizeof (struct tcphdr ) + -sizeof (struct ip6 ); + ip6 = mtod(m, struct ip6 *); + ip6->ip_pl = tlen; + ip6->ip_dst = tcpiph_save.ti_dst6; + ip6->ip_src = tcpiph_save.ti_src6; + ip6->ip_nh = tcpiph_save.ti_nh6; + + (void )ip6_output ((struct socket *)0 , m,0 ); +break ; +default : -/* :TODO:maethor:130321: ? */ +assert (0 );break ; } }
@@ -227,7 +255,7 @@ tcp_newtcpcb(struct socket *so)memset ((char *) tp,0 ,sizeof (struct tcpcb )); tp->seg_next = tp->seg_prev = (struct tcpiphdr *)tp; - tp->t_maxseg =TCP_MSS ; + tp->t_maxseg = (so->so_ffamily ==AF_INET ) ?TCP_MSS :TCP6_MSS ; tp->t_flags =TCP_DO_RFC1323 ? (TF_REQ_SCALE |TF_REQ_TSTMP ) :0 ; tp->t_socket = so;
slirp/tcpip.h +9 -0
@@ -45,6 +45,12 @@ struct tcpiphdr {uint8_t ih_x1;/* (unused) */ uint8_t ih_pr;/* protocol */ } ti_i4; +struct { +struct in6_addr ih_src; +struct in6_addr ih_dst; +uint8_t ih_x1; +uint8_t ih_nh; + } ti_i6; } ti;uint16_t ti_x0;uint16_t ti_len;/* protocol length */
@@ -54,6 +60,9 @@ struct tcpiphdr {#define ti_pr ti.ti_i4.ih_pr#define ti_src ti.ti_i4.ih_src#define ti_dst ti.ti_i4.ih_dst +#define ti_src6 ti.ti_i6.ih_src +#define ti_dst6 ti.ti_i6.ih_dst +#define ti_nh6 ti.ti_i6.ih_nh#define ti_sport ti_t.th_sport#define ti_dport ti_t.th_dport#define ti_seq ti_t.th_seq
Signature · 1 line
2.6.2
PATCH 17/18 slirp: Adding IPv6 address for DNS relay
In reply to Samuel ThibaultFrom: Guillaume Subiron <[email protected]>
This patch adds an IPv6 address to the DNS relay. in6_equal_dns() is developed using this Slirp attribute. sotranslate_in/out/accept() are also updated to manage the IPv6 case so the guest can be able to join the host using one of the Slirp addresses.
--- slirp/ip6.h | 5 ++++- slirp/slirp.c | 2 ++ slirp/slirp.h | 1 + slirp/socket.c | 35 ++++++++++++++++++++++++++++++++--- 4 files changed, 39 insertions(+), 4 deletions(-)
slirp/ip6.h +4 -1
@@ -74,7 +74,10 @@ static inline int in6_equal_mach(struct in6_addr a, struct in6_addr b, || (in6_equal_net(a, (struct in6_addr)LINKLOCAL_ADDR, 64)\ && in6_equal_mach(a, slirp->vhost_addr6, 64))) -#define in6_equal_dns (a) 0 +#define in6_equal_dns (a)\ + ((in6_equal_net(a, slirp->vprefix_addr6, slirp->vprefix_len)\ + || in6_equal_net(a, (struct in6_addr)LINKLOCAL_ADDR, 64))\ + && in6_equal_mach(a, slirp->vnameserver_addr6, slirp->vprefix_len))#define in6_equal_host (a)\ (in6_equal_router(a) || in6_equal_dns(a))
slirp/slirp.c +2 -0
@@ -234,6 +234,8 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, slirp->bootp_filename =g_strdup (bootfile); slirp->vdhcp_startaddr = vdhcp_start; slirp->vnameserver_addr = vnameserver; +/* :TODO:maethor:130311: Use a parameter passed to the function */ +inet_pton (AF_INET6 ,"fec0::3" , &slirp->vnameserver_addr6);if (vdnssearch) {translate_dnssearch (slirp, vdnssearch);
slirp/slirp.h +1 -0
@@ -236,6 +236,7 @@ struct Slirp {struct in6_addr vhost_addr6;struct in_addr vdhcp_startaddr;struct in_addr vnameserver_addr; +struct in6_addr vnameserver_addr6;struct in_addr client_ipaddr;char client_hostname[33 ];
slirp/socket.c +32 -3
@@ -741,12 +741,12 @@ sofwdrain(struct socket *so)/* * Translate addr in host addr when it is a virtual address -* :TODO:maethor:130314: Manage IPv6 */ void sotranslate_out (struct socket *so,struct sockaddr_storage *addr) {Slirp *slirp = so->slirp;struct sockaddr_in *sin = (struct sockaddr_in *)addr; +struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;switch (addr->ss_family) {case AF_INET :
@@ -767,16 +767,29 @@ void sotranslate_out(struct socket *so, struct sockaddr_storage *addr) ntohs(sin->sin_port), inet_ntoa(sin->sin_addr)));break ; +case AF_INET6 : +if (in6_equal_net (so->so_faddr6, slirp->vprefix_addr6, + slirp->vprefix_len)) { +if (in6_equal (so->so_faddr6, slirp->vnameserver_addr6)) { +/*if (get_dns_addr(&addr) < 0) {*/ /* TODO */ + sin6->sin6_addr = in6addr_loopback; +/*}*/ + }else { + sin6->sin6_addr = in6addr_loopback; + } + } +break ; +default :break ; } } -/* :TODO:maethor:130314: IPv6 */ void sotranslate_in (struct socket *so,struct sockaddr_storage *addr) {Slirp *slirp = so->slirp;struct sockaddr_in *sin = (struct sockaddr_in *)addr; +struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;switch (addr->ss_family) {case AF_INET :
@@ -793,6 +806,16 @@ void sotranslate_in(struct socket *so, struct sockaddr_storage *addr) }break ; +case AF_INET6 : +if (in6_equal_net (so->so_faddr6, slirp->vprefix_addr6, + slirp->vprefix_len)) { +if (in6_equal (sin6->sin6_addr, in6addr_loopback) + || !in6_equal (so->so_faddr6, slirp->vhost_addr6)) { + sin6->sin6_addr = so->so_faddr6; + } + } +break ; +default :break ; }
@@ -800,7 +823,6 @@ void sotranslate_in(struct socket *so, struct sockaddr_storage *addr)/* * Translate connections from localhost to the real hostname -* :TODO: IPv6 */ void sotranslate_accept (struct socket *so) {
@@ -815,6 +837,13 @@ void sotranslate_accept(struct socket *so) }break ; +case AF_INET6 : +if (in6_equal (so->so_faddr6, in6addr_any) || +in6_equal (so->so_faddr6, in6addr_loopback)) { + so->so_faddr6 = slirp->vhost_addr6; + } +break ; +default :break ; }
Signature · 1 line
2.6.2
PATCH 18/18 qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses
In reply to Samuel ThibaultFrom: Yann Bordenave <[email protected]>
This patch adds parameters to manage some new options in the qemu -net command.
Slirp IPv6 address, network prefix, and DNS IPv6 address can be given in argument to the qemu command.
Defaults parameters are respectively fec0::2, fec0::, /64 and fec0::3.
--- net/net.c | 31 +++++++++++++++++++++++++++++++ net/slirp.c | 49 +++++++++++++++++++++++++++++++++++++++++++------ qapi-schema.json | 40 ++++++++++++++++++++++++++-------------- qemu-options.hx | 18 ++++++++++++++++-- slirp/libslirp.h | 8 +++++--- slirp/slirp.c | 20 +++++++++----------- 6 files changed, 130 insertions(+), 36 deletions(-)
net/net.c +31 -0
@@ -1049,6 +1049,37 @@ int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)int ret =-1 ; { +/* Parse convenience option format ip6-net=fec0::0[/64] */ +const char *ip6_net =qemu_opt_get (opts,"ip6-net" ); + +if (ip6_net) { +char buf[strlen (ip6_net)+1 ]; + +if (get_str_sep (buf,sizeof (buf), &ip6_net,'/' ) <0 ) { +/* Default 64bit prefix length. */ +qemu_opt_set (opts,"ip6-prefix" , ip6_net, &error_abort); +qemu_opt_set_number (opts,"ip6-prefixlen" ,64 , &error_abort); + }else { +/* User-specified prefix length. */ +int len; +char *end; + +qemu_opt_set (opts,"ip6-prefix" , buf, &error_abort); + len =strtol (ip6_net, &end,10 ); + +if (*end !='\0' ) { +error_setg (errp,QERR_INVALID_PARAMETER_VALUE , +"ip6-prefix" ,"a number" ); + }else { +qemu_opt_set_number (opts,"ip6-prefixlen" , len, + &error_abort); + } + } +qemu_opt_unset (opts,"ip6-net" ); + } + } + + {OptsVisitor *ov =opts_visitor_new (opts);net_visit (opts_get_visitor (ov), is_netdev, &object, &err);
net/slirp.c +43 -6
88 lines not on this page · raw message
qapi-schema.json +26 -14
@@ -2319,6 +2319,14 @@ # @dnssearch: #optional list of DNS suffixes to search, passed as DHCP option # to the guest # +# @ip6-prefix: #optional IPv6 network prefix (since 2.6) +# +# @ip6-prefixlen: #optional IPv6 network prefix length (since 2.6) +# +# @ip6-host: #optional guest-visible IPv6 address of the host (since 2.6) +# +# @ip6-dns: #optional guest-visible IPv6 address of the virtual nameserver (since 2.6) +# # @smb: #optional root directory of the built-in SMB server # # @smbserver: #optional IP address of the built-in SMB server
@@ -2332,20 +2340,24 @@ ## { 'struct': 'NetdevUserOptions', 'data': { - '*hostname': 'str', - '*restrict': 'bool', - '*ip': 'str', - '*net': 'str', - '*host': 'str', - '*tftp': 'str', - '*bootfile': 'str', - '*dhcpstart': 'str', - '*dns': 'str', - '*dnssearch': ['String'], - '*smb': 'str', - '*smbserver': 'str', - '*hostfwd': ['String'], - '*guestfwd': ['String'] } } + '*hostname': 'str', + '*restrict': 'bool', + '*ip': 'str', + '*net': 'str', + '*host': 'str', + '*tftp': 'str', + '*bootfile': 'str', + '*dhcpstart': 'str', + '*dns': 'str', + '*dnssearch': ['String'], + '*ip6-prefix': 'str', + '*ip6-prefixlen': 'int', + '*ip6-host': 'str', + '*ip6-dns': 'str', + '*smb': 'str', + '*smbserver': 'str', + '*hostfwd': ['String'], + '*guestfwd': ['String'] } } ## # @NetdevTapOptions
qemu-options.hx +16 -2
@@ -1486,8 +1486,9 @@ DEF("smb", HAS_ARG, QEMU_OPTION_smb, "", QEMU_ARCH_ALL) DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, #ifdef CONFIG_SLIRP - "-netdev user,id=str[,net=addr[/mask]][,host=addr][,restrict=on|off]\n" - " [,hostname=host][,dhcpstart=addr][,dns=addr][,dnssearch=domain][,tftp=dir]\n" + "-netdev user,id=str[,net=addr[/mask]][,host=addr][,ip6-net=addr[/int]]\n" + " [,ip6-host=addr][,restrict=on|off][,hostname=host][,dhcpstart=addr]\n" + " [,dns=addr][,ip6-dns=addr][,dnssearch=domain][,tftp=dir]\n" " [,bootfile=f][,hostfwd=rule][,guestfwd=rule]" #ifndef _WIN32 "[,smb=dir[,smbserver=addr]]\n"
@@ -1644,6 +1645,14 @@ 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 ip6-net=@var{addr}[/@var{int}] +Set IPv6 network address the guest will see. Optionally specify the prefix +size, as number of valid top-most bits. Default is fec0::/64. + +@item ip6-host=@var{addr} +Specify the guest-visible IPv6 address of the host. Default is the 2nd IPv6 in +the guest network, i.e. xxxx::2. + @item restrict=on|off If this option 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
@@ -1661,6 +1670,11 @@ Specify the guest-visible address of the virtual nameserver. The address must be different from the host address. Default is the 3rd IP in the guest network, i.e. x.x.x.3. +@item ip6-dns=@var{addr} +Specify the guest-visible address of the IPv6 virtual nameserver. The address +must be different from the host address. Default is the 3rd IP in the guest +network, i.e. xxxx::3. + @item dnssearch=@var{domain} Provides an entry for the domain-search list sent by the built-in DHCP server. More than one domain suffix can be transmitted by specifying
slirp/libslirp.h +5 -3
@@ -10,9 +10,11 @@ int get_dns_addr(struct in_addr *pdns_addr);Slirp *slirp_init (int restricted,struct in_addr vnetwork,struct in_addr vnetmask,struct in_addr vhost, -const char *vhostname,const char *tftp_path, -const char *bootfile,struct in_addr vdhcp_start, -struct in_addr vnameserver,const char **vdnssearch, +struct in6_addr vprefix_addr6,uint8_t vprefix_len, +struct in6_addr vhost6,const char *vhostname, +const char *tftp_path,const char *bootfile, +struct in_addr vdhcp_start,struct in_addr vnameserver, +struct in6_addr vnameserver6,const char **vdnssearch,void *opaque);void slirp_cleanup (Slirp *slirp);
slirp/slirp.c +9 -11
@@ -199,9 +199,11 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id);Slirp *slirp_init (int restricted,struct in_addr vnetwork,struct in_addr vnetmask,struct in_addr vhost, -const char *vhostname,const char *tftp_path, -const char *bootfile,struct in_addr vdhcp_start, -struct in_addr vnameserver,const char **vdnssearch, +struct in6_addr vprefix_addr6,uint8_t vprefix_len, +struct in6_addr vhost6,const char *vhostname, +const char *tftp_path,const char *bootfile, +struct in_addr vdhcp_start,struct in_addr vnameserver, +struct in6_addr vnameserver6,const char **vdnssearch,void *opaque) {Slirp *slirp =g_malloc0 (sizeof (Slirp));
@@ -220,12 +222,9 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, slirp->vnetwork_addr = vnetwork; slirp->vnetwork_mask = vnetmask; slirp->vhost_addr = vhost; -/* :TODO:maethor:130311: Use a parameter passed to the function */ -inet_pton (AF_INET6 ,"fec0::0" , &slirp->vprefix_addr6); -/* :TODO:maethor:130311: Use a parameter passed to the function */ - slirp->vprefix_len =64 ; -/* :TODO:maethor:130311: Use a parameter passed to the function */ -inet_pton (AF_INET6 ,"fec0::2" , &slirp->vhost_addr6); + slirp->vprefix_addr6 = vprefix_addr6; + slirp->vprefix_len = vprefix_len; + slirp->vhost_addr6 = vhost6;if (vhostname) {pstrcpy (slirp->client_hostname,sizeof (slirp->client_hostname), vhostname);
@@ -234,8 +233,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, slirp->bootp_filename =g_strdup (bootfile); slirp->vdhcp_startaddr = vdhcp_start; slirp->vnameserver_addr = vnameserver; -/* :TODO:maethor:130311: Use a parameter passed to the function */ -inet_pton (AF_INET6 ,"fec0::3" , &slirp->vnameserver_addr6); + slirp->vnameserver_addr6 = vnameserver6;if (vdnssearch) {translate_dnssearch (slirp, vdnssearch);
Signature · 1 line
2.6.2
Re: [Qemu-devel] [PATCH 01/18] slirp: goto bad in udp_input if sosendto fails
In reply to Samuel ThibaultQuoted 21 lines · Samuel Thibault
From: Guillaume Subiron <[email protected]>
Before this patch, if sosendto fails,
udp_inputis executed as if the packet was sent, recording the packet for icmp errors, which does not makes sense since the packet was not actually sent, errors would be related to a previous packet.This patch adds a goto bad to cut the execution of this function.
Signed-off-by Guillaume Subiron <[email protected]>Signed-off-by Samuel Thibault <[email protected]>--- slirp/debug.h | 2 +- slirp/udp.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)slirp/debug.h +0
-0
slirp/debug.h line 8
Quoted 5 lines
slirp/debug.h +1
-1@@ -5,7 +5,7 @@* terms and conditions of the copyright. */ -//#define DEBUG 1 +#define DEBUG 1
Please don't enable the debug code by default.
slirp/udp.c udp_input(register struct mbuf *m, int iphlen) · line 224
Quoted 12 lines
slirp/udp.c +1
-0@@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen) *ip=save_ip;DEBUG_MISC ((dfd,"udp tx errno = %d-%s\n" ,errno,strerror (errno)));icmp_error (m,ICMP_UNREACH ,ICMP_UNREACH_NET ,0 ,strerror (errno)); + goto bad; }m_free (so->so_m);/* used for ICMP if error on sorecvfrom */
That change looks sane to me.
Thomas
Re: [Qemu-devel] [PATCH 01/18] slirp: goto bad in udp_input if sosendto fails
Quoted 8 lines · Thomas Huth, on Fri 11 Dec 2015 12:54:14 +0100, wrote:
slirp/debug.h line 8
Quoted 6 lines
slirp/debug.h +1
-1@@ -5,7 +5,7 @@* terms and conditions of the copyright. */ -//#define DEBUG 1 +#define DEBUG 1Please don't enable the debug code by default.
Oops, sorry, that wasn't meant to be included in the patch series.
Samuel