All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] [slirp] Make ARP replies at least 64 bytes long
@ 2010-09-13 21:02 Hervé Poussineau
  2010-09-14 22:22 ` Edgar E. Iglesias
  0 siblings, 1 reply; 3+ messages in thread
From: Hervé Poussineau @ 2010-09-13 21:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau

IEEE 802.3 standard requires Ethernet frames to be at least 64 bytes long.
If it is not the case, they will be considered as runt frames, and may be ignored by netcard and/or OS

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 slirp/slirp.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 82fd9b4..2e8c017 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -599,9 +599,12 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
 {
     struct ethhdr *eh = (struct ethhdr *)pkt;
     struct arphdr *ah = (struct arphdr *)(pkt + ETH_HLEN);
-    uint8_t arp_reply[ETH_HLEN + sizeof(struct arphdr)];
-    struct ethhdr *reh = (struct ethhdr *)arp_reply;
-    struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
+    union {
+        uint8_t data[ETH_HLEN + sizeof(struct arphdr)];
+        uint8_t payload[64]; /* Minimum Ethernet frame size */
+    } arp_reply;
+    struct ethhdr *reh = (struct ethhdr *)arp_reply.data;
+    struct arphdr *rah = (struct arphdr *)(arp_reply.data + ETH_HLEN);
     int ar_op;
     struct ex_list *ex_ptr;
 
@@ -619,6 +622,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
             }
             return;
         arp_ok:
+            memset(&arp_reply, 0, sizeof(arp_reply));
             /* XXX: make an ARP request to have the client address */
             memcpy(slirp->client_ethaddr, eh->h_source, ETH_ALEN);
 
@@ -637,7 +641,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
             rah->ar_sip = ah->ar_tip;
             memcpy(rah->ar_tha, ah->ar_sha, ETH_ALEN);
             rah->ar_tip = ah->ar_sip;
-            slirp_output(slirp->opaque, arp_reply, sizeof(arp_reply));
+            slirp_output(slirp->opaque, (const uint8_t *)&arp_reply, sizeof(arp_reply));
         }
         break;
     case ARPOP_REPLY:
-- 
1.7.1.GIT

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] [slirp] Make ARP replies at least 64 bytes long
  2010-09-13 21:02 [Qemu-devel] [PATCH] [slirp] Make ARP replies at least 64 bytes long Hervé Poussineau
@ 2010-09-14 22:22 ` Edgar E. Iglesias
  2010-09-15 19:39   ` Hervé Poussineau
  0 siblings, 1 reply; 3+ messages in thread
From: Edgar E. Iglesias @ 2010-09-14 22:22 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: qemu-devel

On Mon, Sep 13, 2010 at 11:02:42PM +0200, Hervé Poussineau wrote:
> IEEE 802.3 standard requires Ethernet frames to be at least 64 bytes long.
> If it is not the case, they will be considered as runt frames, and may be ignored by netcard and/or OS
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  slirp/slirp.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 82fd9b4..2e8c017 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -599,9 +599,12 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
>  {
>      struct ethhdr *eh = (struct ethhdr *)pkt;
>      struct arphdr *ah = (struct arphdr *)(pkt + ETH_HLEN);
> -    uint8_t arp_reply[ETH_HLEN + sizeof(struct arphdr)];
> -    struct ethhdr *reh = (struct ethhdr *)arp_reply;
> -    struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
> +    union {
> +        uint8_t data[ETH_HLEN + sizeof(struct arphdr)];
> +        uint8_t payload[64]; /* Minimum Ethernet frame size */
> +    } arp_reply;
> +    struct ethhdr *reh = (struct ethhdr *)arp_reply.data;
> +    struct arphdr *rah = (struct arphdr *)(arp_reply.data + ETH_HLEN);


Hi,

Would you mind explaning the point of the union here? Why not just
do something like:

-    uint8_t arp_reply[ETH_HLEN + sizeof(struct arphdr)];
+    uint8_t arp_reply[MAX(ETH_HLEN + sizeof(struct arphdr), 64)];

Cheers


>      int ar_op;
>      struct ex_list *ex_ptr;
>  
> @@ -619,6 +622,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
>              }
>              return;
>          arp_ok:
> +            memset(&arp_reply, 0, sizeof(arp_reply));
>              /* XXX: make an ARP request to have the client address */
>              memcpy(slirp->client_ethaddr, eh->h_source, ETH_ALEN);
>  
> @@ -637,7 +641,7 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
>              rah->ar_sip = ah->ar_tip;
>              memcpy(rah->ar_tha, ah->ar_sha, ETH_ALEN);
>              rah->ar_tip = ah->ar_sip;
> -            slirp_output(slirp->opaque, arp_reply, sizeof(arp_reply));
> +            slirp_output(slirp->opaque, (const uint8_t *)&arp_reply, sizeof(arp_reply));
>          }
>          break;
>      case ARPOP_REPLY:
> -- 
> 1.7.1.GIT
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] [slirp] Make ARP replies at least 64 bytes long
  2010-09-14 22:22 ` Edgar E. Iglesias
@ 2010-09-15 19:39   ` Hervé Poussineau
  0 siblings, 0 replies; 3+ messages in thread
From: Hervé Poussineau @ 2010-09-15 19:39 UTC (permalink / raw)
  To: Edgar E. Iglesias; +Cc: Hervé Poussineau, qemu-devel

Edgar E. Iglesias a écrit :
> On Mon, Sep 13, 2010 at 11:02:42PM +0200, Hervé Poussineau wrote:
>   
>> IEEE 802.3 standard requires Ethernet frames to be at least 64 bytes long.
>> If it is not the case, they will be considered as runt frames, and may be ignored by netcard and/or OS
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  slirp/slirp.c |   12 ++++++++----
>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/slirp/slirp.c b/slirp/slirp.c
>> index 82fd9b4..2e8c017 100644
>> --- a/slirp/slirp.c
>> +++ b/slirp/slirp.c
>> @@ -599,9 +599,12 @@ static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
>>  {
>>      struct ethhdr *eh = (struct ethhdr *)pkt;
>>      struct arphdr *ah = (struct arphdr *)(pkt + ETH_HLEN);
>> -    uint8_t arp_reply[ETH_HLEN + sizeof(struct arphdr)];
>> -    struct ethhdr *reh = (struct ethhdr *)arp_reply;
>> -    struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
>> +    union {
>> +        uint8_t data[ETH_HLEN + sizeof(struct arphdr)];
>> +        uint8_t payload[64]; /* Minimum Ethernet frame size */
>> +    } arp_reply;
>> +    struct ethhdr *reh = (struct ethhdr *)arp_reply.data;
>> +    struct arphdr *rah = (struct arphdr *)(arp_reply.data + ETH_HLEN);
>>     
>
>
> Hi,
>
> Would you mind explaning the point of the union here? Why not just
> do something like:
>
> -    uint8_t arp_reply[ETH_HLEN + sizeof(struct arphdr)];
> +    uint8_t arp_reply[MAX(ETH_HLEN + sizeof(struct arphdr), 64)];
>
>   
Good idea. I'll send an updated patch soon.

Hervé

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-09-15 19:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-13 21:02 [Qemu-devel] [PATCH] [slirp] Make ARP replies at least 64 bytes long Hervé Poussineau
2010-09-14 22:22 ` Edgar E. Iglesias
2010-09-15 19:39   ` Hervé Poussineau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.