[Qemu-devel] [PATCH] slirp: ensure minimum packet size

[Qemu-devel] [PATCH] slirp: ensure minimum packet size

[Bruce Rogers]

With recent gpxe eepro100 drivers, short packets are rejected,
so ensure the minimum ethernet packet size.

Signed-off-by: Bruce Rogers <brogers@novell.com>
---
 slirp/slirp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 332d83b..b611cf7 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -697,7 +697,7 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
         return;
     
     if (!memcmp(slirp->client_ethaddr, zero_ethaddr, ETH_ALEN)) {
-        uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
+        uint8_t arp_req[max(ETH_HLEN + sizeof(struct arphdr), 64)];
         struct ethhdr *reh = (struct ethhdr *)arp_req;
         struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN);
         const struct ip *iph = (const struct ip *)ip_data;
@@ -734,7 +734,7 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
         memcpy(&eh->h_source[2], &slirp->vhost_addr, 4);
         eh->h_proto = htons(ETH_P_IP);
         memcpy(buf + sizeof(struct ethhdr), ip_data, ip_data_len);
-        slirp_output(slirp->opaque, buf, ip_data_len + ETH_HLEN);
+        slirp_output(slirp->opaque, buf, max(ip_data_len + ETH_HLEN, 64));
     }
 }
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH] slirp: ensure minimum packet size

[Anthony Liguori]

On 02/10/2011 11:54 PM, Bruce Rogers wrote:
> With recent gpxe eepro100 drivers, short packets are rejected,
> so ensure the minimum ethernet packet size.
>
> Signed-off-by: Bruce Rogers<brogers@novell.com>
>    

This doesn't make much sense.  I think this is more likely a case where 
we're incorrectly calculating packet size.  Michael fixed an issue 
similar to this in e1000 relating to FCR if I recall correctly.  Maybe 
eepro100 has the same problem?

Regards,

Anthony Liguori

> ---
>   slirp/slirp.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 332d83b..b611cf7 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -697,7 +697,7 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
>           return;
>
>       if (!memcmp(slirp->client_ethaddr, zero_ethaddr, ETH_ALEN)) {
> -        uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
> +        uint8_t arp_req[max(ETH_HLEN + sizeof(struct arphdr), 64)];
>           struct ethhdr *reh = (struct ethhdr *)arp_req;
>           struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN);
>           const struct ip *iph = (const struct ip *)ip_data;
> @@ -734,7 +734,7 @@ void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
>           memcpy(&eh->h_source[2],&slirp->vhost_addr, 4);
>           eh->h_proto = htons(ETH_P_IP);
>           memcpy(buf + sizeof(struct ethhdr), ip_data, ip_data_len);
> -        slirp_output(slirp->opaque, buf, ip_data_len + ETH_HLEN);
> +        slirp_output(slirp->opaque, buf, max(ip_data_len + ETH_HLEN, 64));
>       }
>   }
>
>    

Re: [Qemu-devel] [PATCH] slirp: ensure minimum packet size

[Bruce Rogers]

 >>> On 2/11/2011 at 01:26 AM, Anthony Liguori <anthony@codemonkey.ws> wrote: 
> On 02/10/2011 11:54 PM, Bruce Rogers wrote:
>> With recent gpxe eepro100 drivers, short packets are rejected,
>> so ensure the minimum ethernet packet size.
>>
>> Signed-off-by: Bruce Rogers<brogers@novell.com>
>>    
> 
> This doesn't make much sense.  I think this is more likely a case where 
> we're incorrectly calculating packet size.  Michael fixed an issue 
> similar to this in e1000 relating to FCR if I recall correctly.  Maybe 
> eepro100 has the same problem?
> 

I just took a clue from the other, similar code in slirp.c (arp_input()), where it ensured a minimum of 64 bytes. I now see that that is the wrong approach, and the way this is handled is that the nic emulation adds padding to short packets in its receive handler.

I'll submit a patch with that approach instead.

Thanks for the review.

Bruce