Hi Samuel,
On 9/8/19 1:29 AM, Samuel Thibault wrote:
>
> Now, with MTU set to 9000, the packets just don't go at all. Could you
> try the attached patch? The lowest layer of slirp was indeed limited to
> 1600-byte frames for no good reason. With this and the virtio driver, I
> could exchange 9000-byte packets.
>
> Samuel
>
> diff --git a/src/slirp.c b/src/slirp.c
> index b0194cb..3fd6f68 100644
> --- a/src/slirp.c
> +++ b/src/slirp.c
> @@ -890,20 +890,22 @@ static int if_encap6(Slirp *slirp, struct mbuf
*ifm, struct ethhdr *eh,
> */
> int if_encap(Slirp *slirp, struct mbuf *ifm)
> {
> - uint8_t buf[1600];
> - struct ethhdr *eh = (struct ethhdr *)buf;
> + uint8_t *buf;
> + struct ethhdr *eh;
> uint8_t ethaddr[ETH_ALEN];
> const struct ip *iph = (const struct ip *)ifm->m_data;
> int ret;
>
> - if (ifm->m_len + ETH_HLEN > sizeof(buf)) {
> - return 1;
> - }
> + buf = g_malloc(ifm->m_len + ETH_HLEN);
Since g_malloc() aborts on failure, you want g_try_malloc() here.
> + if (!buf)
> + return 0;
> + eh = (struct ethhdr *)buf;
>
> switch (iph->ip_v) {
> case IPVERSION:
> ret = if_encap4(slirp, ifm, eh, ethaddr);
> if (ret < 2) {
> + g_free(buf);
> return ret;
> }
> break;
> @@ -911,6 +913,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
> case IP6VERSION:
> ret = if_encap6(slirp, ifm, eh, ethaddr);
> if (ret < 2) {
> + g_free(buf);
> return ret;
> }
> break;
> @@ -929,6 +932,7 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
> eh->h_dest[5]);
> memcpy(buf + sizeof(struct ethhdr), ifm->m_data, ifm->m_len);
> slirp_send_packet_all(slirp, buf, ifm->m_len + ETH_HLEN);
> + g_free(buf);
> return 1;
> }
Eventually easier to review using less exit points, i.e.:
-- >8 --
@@ -903,16 +903,10 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
switch (iph->ip_v) {
case IPVERSION:
ret = if_encap4(slirp, ifm, eh, ethaddr);
- if (ret < 2) {
- return ret;
- }
break;
case IP6VERSION:
ret = if_encap6(slirp, ifm, eh, ethaddr);
- if (ret < 2) {
- return ret;
- }
break;
default:
@@ -920,16 +914,21 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
break;
}
- memcpy(eh->h_dest, ethaddr, ETH_ALEN);
- DEBUG_ARG("src = %02x:%02x:%02x:%02x:%02x:%02x", 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_ARG("dst = %02x:%02x:%02x:%02x:%02x:%02x", 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_send_packet_all(slirp, buf, ifm->m_len + ETH_HLEN);
- return 1;
+ if (ret >= 2) {
+ memcpy(eh->h_dest, ethaddr, ETH_ALEN);
+ DEBUG_ARG("src = %02x:%02x:%02x:%02x:%02x:%02x", 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_ARG("dst = %02x:%02x:%02x:%02x:%02x:%02x", 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_send_packet_all(slirp, buf, ifm->m_len + ETH_HLEN);
+ ret = 1;
+ }
+
+ g_free(buf);
+ return ret;
}
---
Anyhow, if you plan to properly (with your S-o-b) commit your patch to
the libslirp repository, using g_try_malloc() you can add:
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Regards,
Phil.
