[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff |
Date: |
Fri, 11 Dec 2015 14:43:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/12/15 14:38, Thomas Huth wrote:
> On 11/12/15 01:15, Samuel Thibault wrote:
>> From: Guillaume Subiron <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Samuel Thibault <address@hidden>
>> ---
>> slirp/mbuf.c | 2 +-
>> slirp/mbuf.h | 2 +-
>> slirp/slirp.c | 24 ++++++++++++++++++++----
>> 3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/slirp/mbuf.c b/slirp/mbuf.c
>> index 795fc29..bc942b6 100644
>> --- a/slirp/mbuf.c
>> +++ b/slirp/mbuf.c
>> @@ -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);
>> diff --git a/slirp/mbuf.h b/slirp/mbuf.h
>> index b144f1c..38fedf4 100644
>> --- a/slirp/mbuf.h
>> +++ b/slirp/mbuf.h
>> @@ -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 {
>> diff --git a/slirp/slirp.c b/slirp/slirp.c
>> index 35f819a..380ddc3 100644
>> --- a/slirp/slirp.c
>> +++ b/slirp/slirp.c
>> @@ -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?
>
>> @@ -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;
>> - }
>> }
>
> 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
- [Qemu-devel] [PATCH 15/18] slirp: Reindent after refactoring, (continued)
- [Qemu-devel] [PATCH 15/18] slirp: Reindent after refactoring, Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 03/18] slirp: Reindent after refactoring, Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 18/18] qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses, Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 10/18] slirp: Adding IPv6, ICMPv6 Echo and NDP autoconfiguration, Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 04/18] slirp: Make Socket structure IPv6 compatible, Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 06/18] slirp: Factorizing and cleaning solookup(), Samuel Thibault, 2015/12/10
- [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Samuel Thibault, 2015/12/10
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Thomas Huth, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Samuel Thibault, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Thomas Huth, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Samuel Thibault, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Thomas Huth, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Laszlo Ersek, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Samuel Thibault, 2015/12/11
- Re: [Qemu-devel] [PATCH 02/18] slirp: Generalizing and neutralizing code before adding IPv6 stuff, Eric Blake, 2015/12/11