Hello,
Just a quick skim over it.
Damien Zammit, le lun. 07 juil. 2025 09:26:54 +0000, a ecrit:
> +int
> +main (int argc, char **argv)
> +{
> +
> + /* Make sure we will not swap out,
> + * because dma buffers for net drivers don't work otherwise */
> + err = wire_task_self ();
> + if (err)
> + error (1, err, "cannot lock all memory");
Will they not? vm_allocate_contiguous makes the allocation non-pageable,
so we shouldn't need to wire the whole rumpnet process (which would
again eat a lot of memory). If we use dma on memory that was not
allocated with vm_allocate_contiguous, rump should be able to wire just
the memory that is being used for dma during the transfer.
> +struct ifreq *
> +search_interface(const char *ifname)
> +{
> + struct ifreq ifr;
> + struct ifreq *dev = NULL;
> + char *last_slash, *name;
> +
> + memset(&ifr, 0, sizeof(ifr));
> + last_slash = strrchr(ifname, '/');
> + if (!last_slash)
> + name = ifname;
> + else
> + name = last_slash + 1;
> + dev = calloc(1, sizeof(*dev));
You can just malloc it, since you overwrite it later on.
> + if (dev == NULL)
> + {
> + mach_print("ERROR: cannot calloc\n");
> + return NULL;
> + }
> + strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
> + if (rump_sys_ioctl(socket_aflink, SIOCGIFFLAGS, &ifr) == -1)
> + {
> + mach_print("siocgifflags failed: ");
> + mach_print(name);
> + mach_print("\n");
> + goto errexit;
> + }
> + *dev = ifr;
> + goto exit;
> +
> +errexit:
> + free(dev);
> + dev = NULL;
> +exit:
> + return dev;
> +}
> +
> +static io_return_t
> +init_interface(struct net_data *nd)
> +{
> + err = rump_sys_ioctl (nd->bpf_fd, BIOCSRTIMEOUT, &timeout);
> + if (err < 0)
> + {
> + mach_print("ERROR: biocsrtimeout failed\n");
> + errno = rump_errno2host(err);
> + return errno;
> + }
Do we really need this? Since we enable BIOCIMMEDIATE we should not need
a timeout, and that will only provide spurious wakeups that eat cpu for
no use.
> +static io_return_t
> +send_packet (struct net_data *nd, io_buf_ptr_t buf, unsigned int bytes)
> +{
> + io_return_t err;
> + struct ethhdr *hdr = (struct ethhdr *)buf;
> + struct iovec iov[2];
> + int result;
> + int flag;
> + int i;
> +
> + if ((ntohs(hdr->h_proto) != ETH_P_IP)
> + && (ntohs(hdr->h_proto) != ETH_P_ARP)
> + && (ntohs(hdr->h_proto) != ETH_P_IPV6))
> + {
> + char proto[16] = {0};
> + snprintf(proto, 16, "0x%04x\n", ntohs(hdr->h_proto));
> + mach_print("ERROR: ethernet frame has wrong protocol: ");
Do we really need to filter out unknown protocols? We'd then have to
change this when porting e.g. STP or whatnot. We can as well just accept
sending anything.
> + mach_print(proto);
> + errno = EPROTO;
> + return -1;
> + }
> +
> + iov[0].iov_base = hdr;
> + iov[0].iov_len = ETH_HLEN;
> + iov[1].iov_base = hdr + 1;
> + iov[1].iov_len = bytes - ETH_HLEN;
Does it not work by just passing hdr in one go? struct ethhdr should
already be ETH_HLEN-sized, and that'll save a copy further down in the
stack.
> + result = rump_sys_writev (nd->bpf_fd, iov, 2);
> + if (result < 0)
> + {
> + errno = EIO;
> + mach_print("ERROR: rump_sys_writev(bpf)\n");
> + return -1;
> + }
> +
> +// for (i = 0; i < bytes; i++)
> +// {
> +// char dmp[4] = {0};
> +// snprintf(dmp, 4, "%02x ", ((uint8_t *)hdr)[i]);
> +// mach_print(dmp);
> +// }
> + return result;
> +}
> +static io_return_t
> +rumpnet_device_write (void *d, mach_port_t reply_port,
> + mach_msg_type_name_t reply_port_type, dev_mode_t mode,
> + recnum_t bn, io_buf_ptr_t data, unsigned int count,
> + int *bytes_written)
> +{
> + struct net_data *nd = (struct net_data *)d;
> + error_t err;
> + char msg[16] = {0};
Unused?
> +
> + err = send_packet(nd, data, count);
> + if (err < 0)
> + return errno;
> + *bytes_written = err;
> +
> + if (*bytes_written != count)
> + {
> + mach_print("ERROR: bytes_written != count\n");
> + return D_IO_ERROR;
> + }
> +
> + vm_deallocate (mach_task_self (), (vm_address_t) data, count);
> +
> + return D_SUCCESS;
> +}
> +
> +static io_return_t
> +device_get_status (void *d, dev_flavor_t flavor, dev_status_t status,
> + mach_msg_type_number_t *count)
> +{
> + struct net_data *nd = (struct net_data *)d;
> + io_return_t err;
> +
> + if (flavor == NET_FLAGS)
> + {
Better use switch/case?
> + if (*count != 1)
> + return D_INVALID_SIZE;
> +
> + err = rump_sys_ioctl(socket_aflink, SIOCGIFFLAGS, nd->dev);
> + if (err < 0)
> + return D_IO_ERROR;
> +
> + *(int *) status = nd->dev->ifr_flags;
> + }
> + else if (flavor == NET_STATUS)
> + {
> + struct net_status *ns = (struct net_status *)status;
> +
> + if (*count < NET_STATUS_COUNT)
> + return D_INVALID_OPERATION;
> +
> + ns->min_packet_size = ETH_HLEN;
> + ns->max_packet_size = SIZEOF_ETH_FRAME;
> + ns->header_format = HDR_ETHERNET;
> + ns->header_size = ETH_HLEN;
> + ns->address_size = ETH_ALEN;
> + ns->flags = nd->dev->ifr_flags;
> + ns->mapped_size = 0;
> +
> + *count = NET_STATUS_COUNT;
> + }
> + else if (flavor == NET_ADDRESS)
> + {
> + err = rump_sys_ioctl(socket_aflink, SIOCGIFFLAGS, nd->dev);
> + if (err < 0)
> + return D_IO_ERROR;
> +
> + err = get_hwaddr(nd->dev->ifr_name, nd->hw_address);
> + if (err)
> + return D_IO_ERROR;
> +
> + status[0] =
> + ((nd->hw_address[0] << 24) |
> + (nd->hw_address[1] << 16) |
> + (nd->hw_address[2] << 8) |
> + (nd->hw_address[3]));
> +
> + status[1] =
> + ((nd->hw_address[4] << 24) |
> + (nd->hw_address[5] << 16));
> +
> + *count = 2;
> + }
> + return D_SUCCESS;
> +}
BTW, does tcpdump work on it? We will definitely want that working :)
Samuel
