[U-Boot] [RFC PATCH v3 14/14] dm: eth: Add a bridge to a real network for sandbox
Joe Hershberger
joe.hershberger at gmail.com
Tue Feb 17 06:16:00 CET 2015
Hi Simon,
On Sun, Feb 15, 2015 at 9:50 AM, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Joe,
>
> On 10 February 2015 at 18:30, Joe Hershberger <joe.hershberger at ni.com>
wrote:
> > Implement a bridge between u-boot's network stack and Linux's raw packet
> > API allowing the sandbox to send and receive packets using the host
> > machine's network interface.
> >
> > This raw Ethernet API requires elevated privileges. You can either run
> > as root, or you can add the capability needed like so:
> >
> > sudo /sbin/setcap "CAP_NET_RAW+ep" u-boot
>
> Can you add a note about thsi in README.sandbox? This seems like a
> major new feature. It was talked about a few years ago when sandbox
> was first created.
OK. Can you maybe point me to that conversation so I can understand what
was anticipated potentially covering more of what was expected.
> >
> > Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> >
> > ---
> >
> > Changes in v3:
> > -Made the os raw packet support for sandbox eth build and work.
> >
> > Changes in v2:
> > -Added the raw packet proof-of-concept patch.
> >
> > arch/sandbox/dts/sandbox.dts | 6 ++
> > arch/sandbox/include/asm/sandbox-raw-os.h | 16 ++++
> > drivers/net/Makefile | 11 +++
> > drivers/net/sandbox-raw-os.c | 105
++++++++++++++++++++++++
> > drivers/net/sandbox-raw.c | 128
++++++++++++++++++++++++++++++
> > include/configs/sandbox.h | 1 +
> > 6 files changed, 267 insertions(+)
> > create mode 100644 arch/sandbox/include/asm/sandbox-raw-os.h
> > create mode 100644 drivers/net/sandbox-raw-os.c
> > create mode 100644 drivers/net/sandbox-raw.c
> >
> > diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
> > index ba635e8..13bd6c2 100644
> > --- a/arch/sandbox/dts/sandbox.dts
> > +++ b/arch/sandbox/dts/sandbox.dts
> > @@ -188,4 +188,10 @@
> > reg = <0x10002000 0x1000>;
> > fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x00>;
> > };
> > +
> > + eth at 80000000 {
> > + compatible = "sandbox,eth,raw";
>
> We normally have "vendor,device", so maybe "sandbox,raw-eth"
OK
> > + reg = <0x80000000 0x1000>;
> > + host-raw-interface = "eth0";
> > + };
> > };
> > diff --git a/arch/sandbox/include/asm/sandbox-raw-os.h
b/arch/sandbox/include/asm/sandbox-raw-os.h
> > new file mode 100644
> > index 0000000..4e5d418
> > --- /dev/null
> > +++ b/arch/sandbox/include/asm/sandbox-raw-os.h
> > @@ -0,0 +1,16 @@
> > +/*
> > + * Copyright (c) 2015 National Instruments
> > + *
> > + * (C) Copyright 2015
> > + * Joe Hershberger <joe.hershberger at ni.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#pragma once
>
> We use #ifdef for this in U-Boot at present. I'm not sure why -
> perhaps compatibility?
OK
> > +
> > +int sandbox_raw_init(int *sd, void **devp, const char *ifname,
> > + unsigned char *ethmac);
> > +int sandbox_raw_send(void *packet, int length, int sd, void *device);
> > +int sandbox_raw_recv(void *packet, int *length, int sd, void *device);
> > +void sandbox_raw_halt(int *sd, void **devp);
>
> Function comments, also what is 'device'?
Comments about what? These are just the functions that implement the
Ethernet driver ops. Is it not sufficient to document the driver ops
themselves (as you requested in the other patch)?
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 15dc431..39975b3 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -51,6 +51,8 @@ obj-$(CONFIG_PCNET) += pcnet.o
> > obj-$(CONFIG_RTL8139) += rtl8139.o
> > obj-$(CONFIG_RTL8169) += rtl8169.o
> > obj-$(CONFIG_ETH_SANDBOX) += sandbox.o
> > +obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw.o
> > +obj-$(CONFIG_ETH_SANDBOX_RAW) += sandbox-raw-os.o
> > obj-$(CONFIG_SH_ETHER) += sh_eth.o
> > obj-$(CONFIG_SMC91111) += smc91111.o
> > obj-$(CONFIG_SMC911X) += smc911x.o
> > @@ -68,3 +70,12 @@ obj-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
xilinx_ll_temac_mdio.o \
> > obj-$(CONFIG_ZYNQ_GEM) += zynq_gem.o
> > obj-$(CONFIG_FSL_MC_ENET) += fsl_mc/
> > obj-$(CONFIG_VSC9953) += vsc9953.o
> > +
> > +# sandbox-raw-os.c is built in the system env, so needs standard
includes
> > +# CFLAGS_REMOVE_sandbox-raw-os.o cannot be used to drop header include
path
> > +quiet_cmd_cc_sandbox-raw-os.o = CC $(quiet_modtag) $@
> > +cmd_cc_sandbox-raw-os.o = $(CC) $(filter-out -nostdinc, \
> > + $(patsubst -I%,-idirafter%,$(c_flags))) -c -o $@ lt;
> > +
> > +$(obj)/sandbox-raw-os.o: $(src)/sandbox-raw-os.c FORCE
> > + $(call if_changed_dep,cc_sandbox-raw-os.o)
>
> Can we please move this to the same directory as os.c, so that all the
> OS-specific stuff is in one place.
OK
> > diff --git a/drivers/net/sandbox-raw-os.c b/drivers/net/sandbox-raw-os.c
> > new file mode 100644
> > index 0000000..43fae60
> > --- /dev/null
> > +++ b/drivers/net/sandbox-raw-os.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * Copyright (c) 2015 National Instruments
> > + *
> > + * (C) Copyright 2015
> > + * Joe Hershberger <joe.hershberger at ni.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#include <errno.h>
> > +#include <net/if.h>
> > +#include <netinet/in.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/types.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/socket.h>
> > +#include <unistd.h>
> > +
> > +#include <linux/if_ether.h>
> > +#include <linux/if_packet.h>
> > +
> > +int sandbox_raw_init(int *sd, void **devp, const char *ifname,
> > + unsigned char *ethmac)
> > +{
> > + int tempsd = 0;
> > + struct ifreq ifr;
> > +
> > + strcpy(ifr.ifr_name, ifname);
> > + ifr.ifr_addr.sa_family = AF_INET;
> > + memset(ethmac, 0, 6);
> > + tempsd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
> > + if (tempsd < 0) {
> > + printf("Failed to open socket: %d %s\n", errno,
> > + strerror(errno));
> > + return 1;
>
> How about:
>
> return -errno
>
> here? That is what U-Boot tends to use now.
OK
> > + }
> > + if (ioctl(tempsd, SIOCGIFHWADDR, &ifr) < 0) {
> > + printf("Failed to call ioctl: %s\n", strerror(errno));
> > + close(tempsd);
> > + return 1;
> > + }
> > + /*
> > + * This only works if the MAC address is overridden with the
actual MAC
> > + * address of the interface being used.
> > + */
> > + memcpy(ethmac, ifr.ifr_hwaddr.sa_data, 6 * sizeof(uint8_t));
> > + close(tempsd);
> > +
> > + *devp = malloc(sizeof(struct sockaddr_ll));
> > + struct sockaddr_ll *device = *devp;
> > + memset(device, 0, sizeof(struct sockaddr_ll));
> > + device->sll_ifindex = if_nametoindex(ifname);
> > + device->sll_family = AF_PACKET;
> > + memcpy(device->sll_addr, ethmac, 6);
> > + device->sll_halen = htons(6);
> > +
> > + *sd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
> > + setsockopt(*sd, SOL_SOCKET, SO_BINDTODEVICE, ifname,
> > + strlen(ifname) + 1);
> > +
> > + return 0;
> > +}
> > +
> > +int sandbox_raw_send(void *packet, int length, int sd, void *device)
> > +{
> > + int retval;
> > +
> > + if (!sd || !device)
> > + return -EINVAL;
> > + retval = sendto(sd, packet, length, 0,
> > + (struct sockaddr *)device, sizeof(struct
sockaddr_ll));
> > + if (retval < 0)
> > + printf("Failed to send packet: %d %s\n", errno,
> > + strerror(errno));
> > + return retval;
> > +}
> > +
> > +int sandbox_raw_recv(void *packet, int *length, int sd, void *device)
> > +{
> > + int retval;
> > + int saddr_size;
> > +
> > + if (!sd || !device)
> > + return -EINVAL;
> > + saddr_size = sizeof(struct sockaddr);
> > + retval = recvfrom(sd, packet, 1536, 0, (struct sockaddr
*)device,
> > + (socklen_t *)&saddr_size);
> > + *length = 0;
> > + if (retval > 0) {
> > + *length = retval;
> > + return 0;
> > + }
> > +
> > + return retval;
> > +}
> > +
> > +void sandbox_raw_halt(int *sd, void **devp)
> > +{
> > + free((struct sockaddr_ll *)*devp);
> > + *devp = NULL;
> > + close(*sd);
> > + *sd = -1;
> > +}
> > diff --git a/drivers/net/sandbox-raw.c b/drivers/net/sandbox-raw.c
> > new file mode 100644
> > index 0000000..90e462a
> > --- /dev/null
> > +++ b/drivers/net/sandbox-raw.c
> > @@ -0,0 +1,128 @@
> > +/*
> > + * Copyright (c) 2015 National Instruments
> > + *
> > + * (C) Copyright 2015
> > + * Joe Hershberger <joe.hershberger at ni.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0+
> > + */
> > +
> > +#include <asm/sandbox-raw-os.h>
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <fdtdec.h>
> > +#include <malloc.h>
> > +#include <net.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +struct eth_sandbox_raw_priv {
> > + int sd;
> > + void *device;
>
> comments
>
> > +};
> > +
> > +static int sb_eth_raw_init(struct udevice *dev, bd_t *bis)
> > +{
> > + debug("eth_sandbox_raw: Init\n");
>
> Put after declarations (perhaps removing code from decls if you like)
> in each case.
>
> > +
> > + struct eth_sandbox_raw_priv *priv = dev->priv;
>
> add blank line
>
> > + if (!priv)
> > + return -EINVAL;
>
> As elsewhere I don't understand why this is needed
>
> > +
> > + struct eth_pdata *pdata = dev->platdata;
> > + int retval;
> > + const char *interface = fdt_getprop(gd->fdt_blob,
dev->of_offset,
> > + "host-raw-interface", NULL);
> > +
> > + retval = sandbox_raw_init(&priv->sd, &priv->device, interface,
> > + pdata->enetaddr);
> > +
> > + return retval;
> > +}
> > +
> > +static int sb_eth_raw_send(struct udevice *dev, void *packet, int
length)
> > +{
> > + debug("eth_sandbox_raw: Send packet %d\n", length);
> > +
> > + struct eth_sandbox_raw_priv *priv = dev->priv;
>
> dev_get_priv(dev) in each case
>
> > +
> > + return sandbox_raw_send(packet, length, priv->sd, priv->device);
> > +}
> > +
> > +static int sb_eth_raw_recv(struct udevice *dev)
> > +{
> > + struct eth_sandbox_raw_priv *priv = dev->priv;
> > +
>
> remove blank line
>
> > + int retval;
> > + uchar buffer[PKTSIZE];
> > + int length;
> > +
> > + if (!priv)
> > + return 0;
> > + retval = sandbox_raw_recv(buffer, &length, priv->sd,
priv->device);
> > + if (!retval && length) {
> > + debug("eth_sandbox_raw: received packet %d\n",
> > + length);
> > + NetReceive(buffer, length);
> > + }
> > + return 0;
> > +}
> > +
> > +static void sb_eth_raw_halt(struct udevice *dev)
> > +{
> > + debug("eth_sandbox_raw: Halt\n");
> > +
> > + struct eth_sandbox_raw_priv *priv = dev->priv;
> > +
> > + if (!priv)
> > + return;
> > + sandbox_raw_halt(&priv->sd, &priv->device);
> > +}
> > +
> > +static int sb_eth_raw_write_hwaddr(struct udevice *dev)
> > +{
> > + struct eth_pdata *pdata = dev->platdata;
> > +
> > + debug("eth_sandbox_raw %s: Write HW ADDR - %pM\n", dev->name,
> > + pdata->enetaddr);
> > + return 0;
> > +}
> > +
> > +static const struct eth_ops sb_eth_raw_ops = {
> > + .init = sb_eth_raw_init,
> > + .send = sb_eth_raw_send,
> > + .recv = sb_eth_raw_recv,
> > + .halt = sb_eth_raw_halt,
> > + .write_hwaddr = sb_eth_raw_write_hwaddr,
> > +};
> > +
> > +static int sb_eth_raw_remove(struct udevice *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF_CONTROL
>
> This is always defined for sandbox.
I figured it was a good pattern to use in general, but if you want me to
prune it out of sandbox-only code I can.
> > +static int sb_eth_raw_ofdata_to_platdata(struct udevice *dev)
> > +{
> > + struct eth_pdata *pdata = dev->platdata;
> > +
> > + pdata->iobase = fdtdec_get_addr(gd->fdt_blob, dev->of_offset,
"reg");
>
> dev_get_addr() is now available in dm/master if you prefer to use that.
OK
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id sb_eth_raw_ids[] = {
> > + { .compatible = "sandbox,eth,raw" },
> > + { }
> > +};
> > +#endif
> > +
> > +U_BOOT_DRIVER(eth_sandbox_raw) = {
> > + .name = "eth_sandbox_raw",
> > + .id = UCLASS_ETH,
> > + .of_match = of_match_ptr(sb_eth_raw_ids),
> > + .ofdata_to_platdata =
of_match_ptr(sb_eth_raw_ofdata_to_platdata),
> > + .remove = sb_eth_raw_remove,
> > + .ops = &sb_eth_raw_ops,
> > + .priv_auto_alloc_size = sizeof(struct eth_sandbox_raw_priv),
> > + .platdata_auto_alloc_size = sizeof(struct eth_pdata),
> > +};
> > diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> > index 9df5f74..7afa3d2 100644
> > --- a/include/configs/sandbox.h
> > +++ b/include/configs/sandbox.h
> > @@ -141,6 +141,7 @@
> >
> > #define CONFIG_DM_ETH
> > #define CONFIG_ETH_SANDBOX
> > +#define CONFIG_ETH_SANDBOX_RAW
>
> These should go in Kconfig. Could be a follow-on patch if that is easier.
OK
> > #define CONFIG_CMD_PING
> >
> > #define CONFIG_CMD_HASH
> > --
> > 1.7.11.5
> >
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list