Patchwork [v2,slirp] Make ARP replies at least 64 bytes long

login
register
mail settings
Submitter Hervé Poussineau
Date Sept. 15, 2010, 8:33 p.m.
Message ID <1284582806-4129-1-git-send-email-hpoussin@reactos.org>
Download mbox | patch
Permalink /patch/64902/
State New
Headers show

Comments

Hervé Poussineau - Sept. 15, 2010, 8:33 p.m.
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 |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Edgar Iglesias - Sept. 17, 2010, 10:56 a.m.
On Wed, Sep 15, 2010 at 10:33:26PM +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>

Applied, thanks.


> ---
>  slirp/slirp.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 82fd9b4..332d83b 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -599,7 +599,7 @@ 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)];
> +    uint8_t arp_reply[max(ETH_HLEN + sizeof(struct arphdr), 64)];
>      struct ethhdr *reh = (struct ethhdr *)arp_reply;
>      struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
>      int ar_op;
> @@ -619,6 +619,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);
>  
> -- 
> 1.7.1.GIT
> 
>
Michael S. Tsirkin - Sept. 20, 2010, 11:43 a.m.
On Wed, Sep 15, 2010 at 10:33:26PM +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>

IMO slirp is not the best place for this, e.g. virtio-net
does not need the padding. 

> ---
>  slirp/slirp.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 82fd9b4..332d83b 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -599,7 +599,7 @@ 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)];
> +    uint8_t arp_reply[max(ETH_HLEN + sizeof(struct arphdr), 64)];
>      struct ethhdr *reh = (struct ethhdr *)arp_reply;
>      struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
>      int ar_op;
> @@ -619,6 +619,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);
>  
> -- 
> 1.7.1.GIT
>
Paolo Bonzini - Sept. 20, 2010, 12:21 p.m.
On 09/20/2010 01:43 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 15, 2010 at 10:33:26PM +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>
>
> IMO slirp is not the best place for this, e.g. virtio-net
> does not need the padding.

I don't think it matters if virtio-net needs it or net; slirp should 
comply to RFC and standards, so it should make frames the correct size.

Paolo

Patch

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 82fd9b4..332d83b 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -599,7 +599,7 @@  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)];
+    uint8_t arp_reply[max(ETH_HLEN + sizeof(struct arphdr), 64)];
     struct ethhdr *reh = (struct ethhdr *)arp_reply;
     struct arphdr *rah = (struct arphdr *)(arp_reply + ETH_HLEN);
     int ar_op;
@@ -619,6 +619,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);