shmc/l4shmnet improvements
Adam Lackorzynski
adam at os.inf.tu-dresden.de
Tue Mar 27 00:45:25 CEST 2012
Hi,
On Tue Mar 20, 2012 at 10:37:58 +0100, Stefan Fritsch wrote:
> the attached shmc.diff fixes many bugs in shmc and adds some new
> functionality. l4shmnet.diff then uses this to implement a protocol that does
> not require one end to wait for the other end during boot, and that allows to
> set the link state up and down. The old protocol is still supported.
Great. I wonder if we still need the old one?
> The changes may be used under the MIT license (which is included in
> the include/shmbuf.h file.
>
> You can find the detailed changelog for shmc.diff below. The diff is on top of
> the diff I have sent in November.
Patch comments inline.
> Add include/shmbuf.h which implements the l4shmnet packet protocol,
> to be used by l4linux and others.
>
> include/shmc.h:
> - l4shmc_iterate_chunk(): New function to iterate over all chunk names
> - l4shmc_chunk_size(): change return value to allow returning an error
> - l4shmc_area_size_free(): New function to get remaining free space
> - l4shmc_area_overhead(): New function to get overhead not usable by
> chunks
> - l4shmc_chunk_overhead(): New function to get size overhead per chunk
>
> include/types.h:
> - Make l4shmc_chunk_desc_t members volatile to avoid races
> - l4shmc_area_t: add _size member for fast access
> - l4shmc_chunk_t: add _capacity member that stores the size outside of
> the shm area and is therefore safe from tampering by remote end
>
> shmc/include/internal.h:
> - l4shmc_chunk_ready(): need HW memory barrier
> - l4shmc_chunk_size(): Check for overflow
> - l4shmc_chunk_capacity() adapt to new _capacity member
> - l4shmc_chunk_consumed(): don't need a memory barrier
>
> lib/src/shmc.c:
> - l4shmc_create(): protect from integer overflows, initialize lock
> - l4shmc_area_overhead(): new function
> - l4shmc_chunk_overhead(): new function
> - next_chunk(): utility function that gets the next chunk, with
> proper sanity checks to avoid invalid memory access/loops if the
> other side is malicious
> - l4chmc_iterate_chunk(): new function
> - l4shmc_add_chunk(): Add sanity checks, use next_chunk(), initialize
> complete chunk struct properly before inserting into linked list
> - l4shmc_area_size_free(): new function
> - l4shmc_get_chunk_to(): Add sanity checks, use next_chunk()
> diff --git a/src/l4linux/arch/l4/kernel/main.c b/src/l4linux/arch/l4/kernel/main.c
> index d71a2b9..6064e4e 100644
> --- a/src/l4linux/arch/l4/kernel/main.c
> +++ b/src/l4linux/arch/l4/kernel/main.c
> @@ -170,6 +170,9 @@ L4_EXTERNAL_FUNC(l4shmc_wait_chunk_to);
> L4_EXTERNAL_FUNC(l4shmc_add_chunk);
> L4_EXTERNAL_FUNC(l4shmc_add_signal);
> L4_EXTERNAL_FUNC(l4shmc_get_signal_to);
> +L4_EXTERNAL_FUNC(l4shmc_iterate_chunk);
> +L4_EXTERNAL_FUNC(l4shmc_area_overhead);
> +L4_EXTERNAL_FUNC(l4shmc_chunk_overhead);
> #endif
>
> #ifdef CONFIG_L4_SERVER
> diff --git a/src/l4linux/drivers/net/l4shmnet.c b/src/l4linux/drivers/net/l4shmnet.c
> index b0f816c..a6ee85d 100644
> --- a/src/l4linux/drivers/net/l4shmnet.c
> +++ b/src/l4linux/drivers/net/l4shmnet.c
> @@ -13,6 +13,7 @@
> #include <asm/generic/do_irq.h>
>
> #include <l4/shmc/shmc.h>
> +#include <l4/shmc/shmbuf.h>
>
> MODULE_AUTHOR("Adam Lackorzynski <adam at os.inf.tu-dresden.de>");
> MODULE_DESCRIPTION("L4shmnet driver");
> @@ -26,6 +27,7 @@ enum {
> static int shmsize = 1 << 20;
>
> static char devs_create[NR_OF_DEVS];
> +static char devs_nocreate[NR_OF_DEVS];
> static char devs_to_add_name[NR_OF_DEVS][20];
> static char devs_to_add_macpart[NR_OF_DEVS];
> static int devs_to_add_pos;
> @@ -41,10 +43,11 @@ struct l4x_l4shmc_priv {
> l4shmc_chunk_t tx_chunk;
> l4shmc_chunk_t rx_chunk;
>
> - char *tx_ring_start;
> - char *rx_ring_start;
> - unsigned long tx_ring_size;
> - unsigned long rx_ring_size;
> + struct shmbuf sb;
> + unsigned int num:8;
> + unsigned int remote_attached:1;
> + unsigned int link_up:1;
> + unsigned int compat:1;
> };
>
> struct l4x_l4shmc_netdev {
> @@ -62,69 +65,231 @@ struct ring_chunk_head {
> unsigned long size; // 0 == not used, >= 0 valid chunk
> };
>
> -static inline int chunk_size(l4shmc_area_t *s)
> +/****************************************************************************
> + * For attaching to the other side, there are two methods:
> + *
> + * - Old, compatible protocol (selected with parameter 'create' and
> + * 'nocreate'):
> + * 1. There is a designated master who sets up the shm segment and
> + * his tx chunk and tx sig, named "joe". He then waits for the "bob"
> + * chunk and sig to appear.
> + * 2. The slave polls for the shm segment to appear, creates his
> + * tx chunk and tx sig, named "bob". He then connects the "joe"
> + * chunk and sig to his rx side.
> + * 3. The master connects "bob" chunk/sig to his rx side.
> + * After that, the link is always up and there is no way to change it to
> + * down again.
> + *
> + * - New protocol (if neither 'create' nor 'nocreate' is given):
> + * 1. Both parties will try to create the shm segment. The first one wins.
> + * 2. Both parties create a chunk and sig named after their MAC address
> + * and use it as their rx side. They will set the chunk state to "clear"
> + * which means "link down".
> + * 3. Both parties will look for another chunk. If there is one, they will
> + * connect it as their tx side and trigger that signal.
> + * 4. The first one won't yet find the other's chunk but he will be alerted
> + * by the second's signal and then try again. He will then connect the
> + * chunk and sig as his tx side, and trigger the signal.
> + * Due to the signal, there is no need to actively poll for the arrival of
> + * the second partner.
> + * If one partner wants to set the interface up, he sets the state of his
> + * rx chunk to "ready" and triggers the tx signal. If both chunks are
> + * "ready" the link state is defined to be "up".
> + * The interface state can be set down again in the same way.
> + * The MAC addresses of both parties must be different.
> + *
> + * When the link is up, both variants use the same protocol for exchanging
> + * packets.
> + ****************************************************************************/
> +
> +static inline int chunk_size(l4shmc_area_t *s, int compat)
> {
> - return (l4shmc_area_size(s) / 2) - 52;
> + if (compat)
> + return (l4shmc_area_size(s) / 2) - 52;
> + else
> + return (l4shmc_area_size(s) - l4shmc_area_overhead()) / 2 -
> + l4shmc_chunk_overhead();
> }
>
> -static int l4x_l4shmc_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
> +static int init_dev_compat(struct net_device *dev)
> {
> - struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> - short length = skb->len;
> - struct chunk_head *chhead;
> - struct ring_chunk_head *rph;
> - unsigned long l, offs, nextoffs, r;
> - L4XV_V(f);
> + struct l4x_l4shmc_priv *priv = netdev_priv(dev);
> + int num = priv->num;
> + int err = 0;
>
> - if (length == 0)
> - return 0;
> + if ((err = l4shmc_add_chunk(&priv->shmcarea, devs_create[num] ? "joe" : "bob",
> + chunk_size(&priv->shmcarea, 1), &priv->tx_chunk)))
> + return err;
>
> - // copy chunk into the ring
> - chhead = (struct chunk_head *)l4shmc_chunk_ptr(&priv->tx_chunk);
> + if ((err = l4shmc_add_signal(&priv->shmcarea, devs_create[num] ? "joe" : "bob",
> + &priv->tx_sig)))
> + return err;
> +
> + if ((err = l4shmc_connect_chunk_signal(&priv->tx_chunk, &priv->tx_sig)))
> + return err;
> +
> + /* Now get the receiving side */
> + if ((err = l4shmc_get_chunk_to(&priv->shmcarea, devs_create[num] ? "bob" : "joe",
> + WAIT_TIMEOUT, &priv->rx_chunk))) {
> + printk("%s: Did not find other side\n", devs_to_add_name[num]);
> + return err;
> + }
> +
> + if ((err = l4shmc_get_signal_to(&priv->shmcarea, devs_create[num] ? "bob" : "joe",
> + WAIT_TIMEOUT, &priv->rx_sig))) {
> + printk("%s: Could not get signal\n", devs_to_add_name[num]);
> + return err;
> + }
> + if ((err = l4shmc_connect_chunk_signal(&priv->rx_chunk, &priv->rx_sig)))
> + return err;
>
> - offs = chhead->next_offs_to_write;
> + shmbuf_init(&priv->sb, l4shmc_chunk_ptr(&priv->rx_chunk),
> + l4shmc_chunk_capacity(&priv->rx_chunk),
> + l4shmc_chunk_ptr(&priv->tx_chunk),
> + l4shmc_chunk_capacity(&priv->tx_chunk));
>
> - rph = (struct ring_chunk_head *)(priv->tx_ring_start + offs);
> + priv->remote_attached = 1;
>
> - BUILD_BUG_ON(sizeof(struct ring_chunk_head) & (sizeof(struct ring_chunk_head) - 1));
> + return err;
> +}
>
> - nextoffs = (offs + length + sizeof(struct ring_chunk_head) + sizeof(struct ring_chunk_head) - 1)
> - & ~(sizeof(struct ring_chunk_head) - 1);
> +static int init_dev_self(struct net_device *dev)
> +{
> + struct l4x_l4shmc_priv *priv = netdev_priv(dev);
> + int ret;
> + char myname[15];
> +
> + snprintf(myname, sizeof(myname), "%02x%02x%02x%02x%02x%02x",
> + dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
> + dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
There's a %pm format specifier.
> +
> + if ((ret = l4shmc_add_chunk(&priv->shmcarea, myname,
> + chunk_size(&priv->shmcarea, 0),
> + &priv->rx_chunk))) {
> + if (ret != -L4_EEXIST) {
> + printk(KERN_WARNING "%s: Can't create chunk: %d",
> + dev->name, ret);
> + return ret;
> + }
> + if ((ret = l4shmc_get_chunk(&priv->shmcarea, myname,
> + &priv->rx_chunk))) {
> + printk(KERN_WARNING "%s: Can't attach to existing chunk '%s': %d",
> + dev->name, myname, ret);
> + return ret;
> + }
> + }
>
> - r = chhead->next_offs_to_read;
> - if (r <= offs)
> - r += priv->tx_ring_size;
> - if (nextoffs >= r) {
> - chhead->writer_blocked = 1;
> - netif_stop_queue(netdev);
> - return 1;
> + /*
> + * The chunk ready / chunk cleared flag is abused as ifup/ifdown flag:
> + * cleared == consumed -> if down
> + * ready -> if up
> + */
> + l4shmc_chunk_consumed(&priv->rx_chunk);
> +
> + if ((ret = l4shmc_add_signal(&priv->shmcarea, myname, &priv->rx_sig)))
> + return ret;
> +
> + if ((ret = l4shmc_connect_chunk_signal(&priv->rx_chunk, &priv->rx_sig)))
> + return ret;
> +
> + return 0;
> +}
> +
> +static int init_dev_other(struct net_device *dev)
> +{
> + struct l4x_l4shmc_priv *priv = netdev_priv(dev);
> + char myname[15];
> + char othername[15];
> + const char *ptr;
> + long offs = 0;
> + int ret;
> +
> + othername[0] = '\0';
> + snprintf(myname, sizeof(myname), "%02x%02x%02x%02x%02x%02x",
> + dev->dev_addr[0], dev->dev_addr[1], dev->dev_addr[2],
> + dev->dev_addr[3], dev->dev_addr[4], dev->dev_addr[5]);
%pm
> +
> + while ((offs = l4shmc_iterate_chunk(&priv->shmcarea, &ptr, offs)) > 0) {
> + if (strncmp(ptr, myname, sizeof(myname)) != 0) {
> + strncpy(othername, ptr, sizeof(othername));
> + if (othername[sizeof(othername)-1] != '\0') {
> + /* name too long or invalid */
> + return -L4_EIO;
> + }
> + }
> }
> + if (offs < 0)
> + return offs;
> + if (!othername[0])
> + return -L4_EAGAIN;
> +
> + if ((ret = l4shmc_get_chunk(&priv->shmcarea, othername,
> + &priv->tx_chunk)))
> + return ret;
>
> - nextoffs %= priv->tx_ring_size;
> + if ((ret = l4shmc_get_signal_to(&priv->shmcarea, othername,
> + WAIT_TIMEOUT, &priv->tx_sig)))
> + return ret;
>
> - offs += sizeof(struct ring_chunk_head);
> - offs %= priv->tx_ring_size;
> + shmbuf_init(&priv->sb, l4shmc_chunk_ptr(&priv->rx_chunk),
> + l4shmc_chunk_capacity(&priv->rx_chunk),
> + l4shmc_chunk_ptr(&priv->tx_chunk),
> + l4shmc_chunk_capacity(&priv->tx_chunk));
>
> - if (offs + length > priv->tx_ring_size)
> - l = priv->tx_ring_size - offs;
> + priv->remote_attached = 1;
> + l4shmc_trigger(&priv->tx_sig);
> + printk(KERN_INFO "%s: L4ShmNet established, with %pM, IRQ %d\n",
> + dev->name, dev->dev_addr, dev->irq);
> +
> + return 0;
> +}
> +
> +static void update_carrier(struct net_device *dev)
> +{
> + struct l4x_l4shmc_priv *priv = netdev_priv(dev);
> + int link_up;
> +
> + if (priv->compat)
> + link_up = 1;
> + else if (priv->remote_attached &&
> + l4shmc_is_chunk_ready(&priv->tx_chunk) &&
> + l4shmc_is_chunk_ready(&priv->rx_chunk))
> + link_up = 1;
> else
> - l = length;
> + link_up = 0;
> + if (priv->link_up == link_up)
> + return;
> + priv->link_up = link_up;
> + if (link_up) {
> + netif_carrier_on(dev);
> + netif_wake_queue(dev);
> + }
> + else {
> + netif_carrier_off(dev);
> + netif_stop_queue(dev);
> + }
> +}
>
> - memcpy(priv->tx_ring_start + offs, (char *)skb->data, l);
> - if (l != length)
> - memcpy(priv->tx_ring_start, (char *)skb->data + l, length - l);
> +static int l4x_l4shmc_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
> +{
> + struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> + int ret;
> + L4XV_V(f);
> +
> + ret = shmbuf_tx(&priv->sb, skb->data, skb->len);
> + if (ret == -L4_EAGAIN) {
> + netif_stop_queue(netdev);
> + return 1;
> + }
> + /* XXX: handle other shmbuf_tx errors */
>
> - // now write to shm
> - rph->size = length;
> - rph = (struct ring_chunk_head *)(priv->tx_ring_start + nextoffs);
> - rph->size = 0;
> - chhead->next_offs_to_write = nextoffs;
> wmb();
>
> L4XV_L(f);
> l4shmc_trigger(&priv->tx_sig);
> L4XV_U(f);
>
> +
> netdev->trans_start = jiffies;
> priv->net_stats.tx_packets++;
> priv->net_stats.tx_bytes += skb->len;
> @@ -149,8 +314,18 @@ static irqreturn_t l4x_l4shmc_interrupt(int irq, void *dev_id)
> struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> struct sk_buff *skb;
> struct chunk_head *chhead;
> - struct ring_chunk_head *rph;
> - unsigned long offs;
> + unsigned long len;
> + int ret;
> +
> + if (!priv->remote_attached && !priv->compat) {
> + L4XV_V(f);
> + L4XV_L(f);
> + init_dev_other(netdev);
> + L4XV_U(f);
Please lock inside init_dev_other.
> + }
> +
> + update_carrier(netdev);
> + /* XXX: return if link down? */
>
> chhead = (struct chunk_head *)l4shmc_chunk_ptr(&priv->tx_chunk);
> if (chhead->writer_blocked) {
> @@ -158,48 +333,23 @@ static irqreturn_t l4x_l4shmc_interrupt(int irq, void *dev_id)
> netif_wake_queue(netdev);
> }
>
> - chhead = (struct chunk_head *)l4shmc_chunk_ptr(&priv->rx_chunk);
> - offs = chhead->next_offs_to_read;
> - while (1) {
> - unsigned long l;
> + while ((len = shmbuf_rx_len(&priv->sb)) > 0) {
> char *p;
>
> - rph = (struct ring_chunk_head *)(priv->rx_ring_start + offs);
> -
> - if (!rph->size)
> - break;
> -
> - skb = dev_alloc_skb(rph->size);
> + skb = dev_alloc_skb(len);
In the current version this is dev_alloc_skb(rph->size + NET_IP_ALIGN),
it's probably dev_alloc_skb(len + NET_IP_ALIGN)?
> if (unlikely(!skb)) {
> printk(KERN_WARNING "%s: dropping packet (%ld).\n",
> - netdev->name, rph->size);
> + netdev->name, len);
> priv->net_stats.rx_dropped++;
> break;
> }
>
> skb->dev = netdev;
> -
> - offs += sizeof(struct ring_chunk_head);
> - offs %= priv->rx_ring_size;
> -
> - if (offs + rph->size > priv->rx_ring_size)
> - l = priv->rx_ring_size - offs;
> - else
> - l = rph->size;
> -
> skb_reserve(skb, NET_IP_ALIGN);
> - p = skb_put(skb, rph->size);
> - memcpy(p, priv->rx_ring_start + offs, l);
> - if (l != rph->size)
> - memcpy(p + l, priv->rx_ring_start, rph->size - l);
> -
> - offs = (offs + rph->size + sizeof(struct ring_chunk_head) - 1)
> - & ~(sizeof(struct ring_chunk_head) - 1);
> - offs %= priv->rx_ring_size;
> - chhead->next_offs_to_read = offs;
> - rph->size = 0;
> -
> + p = skb_put(skb, len);
> + ret = shmbuf_rx(&priv->sb, p, len);
> + /* XXX: check errors */
> skb->protocol = eth_type_trans(skb, netdev);
> netif_rx(skb);
>
> @@ -208,19 +358,22 @@ static irqreturn_t l4x_l4shmc_interrupt(int irq, void *dev_id)
> priv->net_stats.rx_packets++;
> }
>
> - if (chhead->writer_blocked) {
> + if (len == -L4_EAGAIN) {
> L4XV_V(f);
> L4XV_L(f);
> l4shmc_trigger(&priv->tx_sig);
> L4XV_U(f);
> }
> + /* XXX: handle other errors */
>
> return IRQ_HANDLED;
> }
>
> static int l4x_l4shmc_open(struct net_device *netdev)
> {
> + struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> int err;
> + L4XV_V(f);
>
> netif_carrier_off(netdev);
>
> @@ -232,27 +385,38 @@ static int l4x_l4shmc_open(struct net_device *netdev)
> return err;
> }
>
> -
> - netif_carrier_on(netdev);
> - netif_wake_queue(netdev);
> + l4shmc_chunk_ready(&priv->rx_chunk, 0);
> + L4XV_L(f);
> + l4shmc_trigger(&priv->tx_sig);
> + L4XV_U(f);
> + update_carrier(netdev);
>
> return 0;
> }
>
> static int l4x_l4shmc_close(struct net_device *netdev)
> {
> + struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> + L4XV_V(f);
> +
> free_irq(netdev->irq, netdev);
> - netif_stop_queue(netdev);
> - netif_carrier_off(netdev);
> + l4shmc_chunk_consumed(&priv->rx_chunk);
> + L4XV_L(f);
> + l4shmc_trigger(&priv->tx_sig);
> + L4XV_U(f);
> + update_carrier(netdev);
>
> return 0;
> }
>
> +#define MIN(a,b) ((a) < (b) ? (a) : (b))
> static int l4x_l4shmc_change_mtu(struct net_device *netdev, int new_mtu)
> {
> struct l4x_l4shmc_priv *priv = netdev_priv(netdev);
> + int max_mtu = MIN(l4shmc_chunk_capacity(&priv->rx_chunk),
> + l4shmc_chunk_capacity(&priv->tx_chunk)) - 100;
There's a min 'function' in kernel.h (not doing double evaluation).
>
> - if (new_mtu > chunk_size(&priv->shmcarea) - 100)
> + if (new_mtu > max_mtu)
> return -EINVAL;
>
> netdev->mtu = new_mtu;
> @@ -268,13 +432,13 @@ static const struct net_device_ops l4shmnet_netdev_ops = {
>
> };
>
> -static int __init l4x_l4shmnet_init_dev(int num, const char *name)
> +static int __init l4x_l4shmnet_init_dev(int num)
> {
> struct l4x_l4shmc_priv *priv;
> struct net_device *dev = NULL;
> struct l4x_l4shmc_netdev *nd = NULL;
> - struct chunk_head *ch;
> - int err;
> + const char *name = devs_to_add_name[num];
> + int err, ret;
> L4XV_V(f);
>
> if (shmsize < PAGE_SIZE)
> @@ -284,72 +448,49 @@ static int __init l4x_l4shmnet_init_dev(int num, const char *name)
> return -ENOMEM;
>
> dev->netdev_ops = &l4shmnet_netdev_ops,
> + dev->dev_addr[0] = 0x52;
> + dev->dev_addr[1] = 0x54;
> + dev->dev_addr[2] = 0x00;
> + dev->dev_addr[3] = 0xb0;
> + dev->dev_addr[4] = 0xcf;
> + dev->dev_addr[5] = devs_to_add_macpart[num];
> +
> priv = netdev_priv(dev);
> + priv->num = num;
> + priv->link_up = 0;
> + priv->remote_attached = 0;
> + priv->compat = (devs_create[num]||devs_nocreate[num]);
>
> - printk("%s: Requesting, role %s, Shmsize %d Kbytes\n",
> + printk("%s: Requesting, %s, Shmsize %d Kbytes\n",
> name,
> - devs_create[num] ? "Creator" : "User", shmsize >> 10);
> + (!priv->compat) ? "auto-create" :
> + devs_create[num] ? "Creator (compat)" : "User (compat)",
> + shmsize >> 10);
>
> L4XV_L(f);
> err = -ENOMEM;
> - if (devs_create[num]) {
> - if (l4shmc_create(name, shmsize))
> - goto err_out_free_dev_unlock;
> + if (devs_create[num] || !priv->compat) {
> + if ((ret = l4shmc_create(name, shmsize))) {
> + if (priv->compat || ret != -L4_EEXIST)
> + goto err_out_free_dev_unlock;
> + }
> }
>
> // we block very long here, don't do that
> if (l4shmc_attach_to(name, WAIT_TIMEOUT, &priv->shmcarea))
> goto err_out_free_dev_unlock;
>
> - if (l4shmc_add_chunk(&priv->shmcarea, devs_create[num] ? "joe" : "bob",
> - chunk_size(&priv->shmcarea), &priv->tx_chunk))
> - goto err_out_free_dev_unlock;
> -
> - if (l4shmc_add_signal(&priv->shmcarea, devs_create[num] ? "joe" : "bob",
> - &priv->tx_sig))
> - goto err_out_free_dev_unlock;
> -
> - if (l4shmc_connect_chunk_signal(&priv->tx_chunk, &priv->tx_sig))
> - goto err_out_free_dev_unlock;
> -
> - /* Now get the receiving side */
> - if (l4shmc_get_chunk_to(&priv->shmcarea, devs_create[num] ? "bob" : "joe",
> - WAIT_TIMEOUT, &priv->rx_chunk)) {
> - printk("%s: Did not find other side\n", name);
> + if (priv->compat)
> + ret = init_dev_compat(dev);
> + else
> + ret = init_dev_self(dev);
> + if (ret < 0) {
> + /* XXX: convert error code */
> goto err_out_free_dev_unlock;
> }
>
> - if (l4shmc_get_signal_to(&priv->shmcarea, devs_create[num] ? "bob" : "joe",
> - WAIT_TIMEOUT, &priv->rx_sig)) {
> - printk("%s: Could not get signal\n", name);
> - goto err_out_free_dev_unlock;
> - }
> - if (l4shmc_connect_chunk_signal(&priv->rx_chunk, &priv->rx_sig))
> - goto err_out_free_dev_unlock;
> L4XV_U(f);
>
> - ch = (struct chunk_head *)l4shmc_chunk_ptr(&priv->tx_chunk);
> - ch->next_offs_to_write = 0;
> - ch->next_offs_to_read = 0;
> - ch->writer_blocked = 0;
> -
> - priv->tx_ring_size = l4shmc_chunk_capacity(&priv->tx_chunk)
> - - sizeof(struct chunk_head);
> - priv->rx_ring_size = l4shmc_chunk_capacity(&priv->rx_chunk)
> - - sizeof(struct chunk_head);
> -
> - priv->tx_ring_start = (char *)l4shmc_chunk_ptr(&priv->tx_chunk)
> - + sizeof(struct chunk_head);
> - priv->rx_ring_start = (char *)l4shmc_chunk_ptr(&priv->rx_chunk)
> - + sizeof(struct chunk_head);
> -
> - dev->dev_addr[0] = 0x52;
> - dev->dev_addr[1] = 0x54;
> - dev->dev_addr[2] = 0x00;
> - dev->dev_addr[3] = 0xb0;
> - dev->dev_addr[4] = 0xcf;
> - dev->dev_addr[5] = devs_to_add_macpart[num];
> -
> if ((dev->irq = l4x_register_irq(l4shmc_signal_cap(&priv->rx_sig))) < 0) {
> printk("Failed to get virq\n");
> goto err_out_free_dev;
> @@ -371,8 +512,18 @@ static int __init l4x_l4shmnet_init_dev(int num, const char *name)
> nd->dev = dev;
> list_add(&nd->list, &l4x_l4shmnet_netdevices);
>
> - printk(KERN_INFO "%s: L4ShmNet established, with %pM, IRQ %d\n",
> - dev->name, dev->dev_addr, dev->irq);
> + if (priv->compat)
> + printk(KERN_INFO "%s: L4ShmNet established (compat), with %pM, IRQ %d\n",
> + dev->name, dev->dev_addr, dev->irq);
> + else {
> + L4XV_L(f);
> + ret = init_dev_other(dev);
> + L4XV_U(f);
Please lock inside.
> + if (ret < 0 && ret != -L4_EAGAIN) {
> + /* XXX: convert error code */
> + goto err_out_free_dev;
> + }
> + }
>
> return 0;
>
> @@ -391,7 +542,7 @@ static int __init l4x_l4shmnet_init(void)
>
> for (i = 0; i < devs_to_add_pos; ++i)
> if (*devs_to_add_name[i]
> - && !l4x_l4shmnet_init_dev(i, devs_to_add_name[i]))
> + && !l4x_l4shmnet_init_dev(i))
> ret = 0;
> return ret;
> }
> @@ -435,19 +586,25 @@ static int l4x_l4shmnet_setup(const char *val, struct kernel_param *kp)
> do {
> if (!strncmp(c + 1, "create", 6))
> devs_create[devs_to_add_pos] = 1;
> + else if (!strncmp(c + 1, "nocreate", 8))
> + devs_nocreate[devs_to_add_pos] = 1;
> else if (!strncmp(c + 1, "macpart=", 8)) {
> devs_to_add_macpart[devs_to_add_pos]
> = simple_strtoul(c + 9, NULL, 0);
> }
> } while ((c = strchr(c + 1, ',')));
> }
> + if (devs_create[devs_to_add_pos] && devs_nocreate[devs_to_add_pos]) {
> + printk("l4shmnet: create and nocreate are mutually exclusive");
> + return 1;
> + }
> strlcpy(devs_to_add_name[devs_to_add_pos], val, l);
> devs_to_add_pos++;
> return 0;
> }
>
> module_param_call(add, l4x_l4shmnet_setup, NULL, NULL, 0200);
> -MODULE_PARM_DESC(add, "Use l4shmnet.add=name,macpart[,create] to add a device, name queried in namespace");
> +MODULE_PARM_DESC(add, "Use l4shmnet.add=name,macpart[,[no]create] to add a device, name queried in namespace");
>
> module_param(shmsize, int, 0);
> MODULE_PARM_DESC(shmsize, "Size of the shared memory area");
> diff --git a/src/l4/pkg/shmc/include/internal.h b/src/l4/pkg/shmc/include/internal.h
> index c4cb4bf..7c9758c 100644
> --- a/src/l4/pkg/shmc/include/internal.h
> +++ b/src/l4/pkg/shmc/include/internal.h
> @@ -91,7 +91,7 @@ L4_CV L4_INLINE long
> l4shmc_chunk_ready(l4shmc_chunk_t *chunk, l4_umword_t size)
> {
> chunk->_chunk->_size = size;
> - asm volatile("" : : : "memory");
> + __sync_synchronize();
> chunk->_chunk->_status = L4SHMC_CHUNK_READY;
> return L4_EOK;
> }
> @@ -128,16 +128,19 @@ l4shmc_signal_cap(l4shmc_signal_t *signal)
> return signal->_sigcap;
> }
>
> -L4_CV L4_INLINE l4_umword_t
> +L4_CV L4_INLINE long
> l4shmc_chunk_size(l4shmc_chunk_t *p)
> {
> - return p->_chunk->_size;
> + l4_umword_t s = p->_chunk->_size;
> + if (s > p->_capacity)
> + return -L4_EIO;
> + return s;
> }
>
> -L4_CV L4_INLINE l4_umword_t
> +L4_CV L4_INLINE long
> l4shmc_chunk_capacity(l4shmc_chunk_t *p)
> {
> - return p->_chunk->_capacity;
> + return p->_capacity;
> }
>
> L4_CV L4_INLINE long
> @@ -152,7 +155,6 @@ l4shmc_chunk_try_to_take(l4shmc_chunk_t *chunk)
> L4_CV L4_INLINE long
> l4shmc_chunk_consumed(l4shmc_chunk_t *chunk)
> {
> - asm volatile("" : : : "memory");
> chunk->_chunk->_status = L4SHMC_CHUNK_CLEAR;
> return L4_EOK;
> }
> diff --git a/src/l4/pkg/shmc/include/shmbuf.h b/src/l4/pkg/shmc/include/shmbuf.h
> new file mode 100644
> index 0000000..dbee89d
> --- /dev/null
> +++ b/src/l4/pkg/shmc/include/shmbuf.h
> @@ -0,0 +1,638 @@
> +/*
> + * Copyright (c) 2011 Stefan Fritsch <stefan_fritsch at genua.de>
> + * Christian Ehrhardt <christian_ehrhardt at genua.de>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +#ifndef L4SHMC_SHMBUF_H
> +#define L4SHMC_SHMBUF_H
> +
> +#include <l4/shmc/shmc.h>
> +#include <l4/sys/err.h>
> +
#include <l4/sys/compiler.h>
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
We also have EXTERN_C_BEGIN (and EXTERN_C_END)
> +
> +/* sizeof(struct pkt_head) must be power of two */
> +struct pkt_head {
> + unsigned long size;
> +};
static_assert((sizeof(struct pkt_head) & ~sizeof(struct pkt_head)) == 0,
"sizeof(struct pkt_head) not power of 2");
Admittedly this works only within function bodies but might be useful
anyways.
Then, since this is a header file, please prefix everything non-local
with 'l4shm_' to avoid any possible name clashes.
> +
> +struct shm_chunk_head {
> + /** end of ring content */
> + volatile unsigned long next_offs_to_write;
> + /** start of ring content */
> + volatile unsigned long next_offs_to_read;
> + /** ring buffer full */
> + volatile unsigned long writer_blocked;
> + /** The packet buffers. */
> + volatile struct pkt_head pkg[0];
> +};
> +
> +#if 0
> +#define debug_printf(...) printf(__VA_ARGS__)
> +#else
> +#define debug_printf(...)
> +#endif
Please no such macros. First I'd like those always parsed so that
they do not break: if (0) ... (or if (compile-time-fix-expression) ...).
Then, debug_printf is just too generic.
> +
> +/**
> + * Return the size of the largest packet that currently fits into
> + * the given chunk.
> + * @param chunk The chunk.
> + * @param chunksize The size of the data area of the chunk (maintained
> + * outside of the shared memory area).
> + * It is the callers responsibility to ensure that it is the sender of
> + * this chunk.
> + */
> +static inline unsigned long shmchunk_tx_free(struct shm_chunk_head *chunk,
> + unsigned long chunksize)
Use L4_INLINE, it will do the right thing depending on the compiler.
> +{
> + unsigned long roff = chunk->next_offs_to_read;
> + unsigned long woff = chunk->next_offs_to_write;
> + unsigned long space;
> +
> + if (woff >= chunksize || roff >= chunksize)
> + return 0;
> +
> + if (woff < roff)
> + space = roff - woff;
> + else
> + space = chunksize - (woff - roff);
> + if (space < 2 * sizeof(struct pkt_head))
> + return 0;
> + return space - 2 * sizeof(struct pkt_head);
> +}
> +
> +/** align v to power of 2 boundary */
> +#define SHMC_ALIGN(v, boundary) \
> + (((unsigned long)(v) + ((unsigned long)(boundary)-1)) \
> + & ~((unsigned long)(boundary) - 1))
Please do not use macros but inline functions.
> +#define SHMC_ALIGN_PTR_PH(v) \
> + ((struct pkt_head *)SHMC_ALIGN(v, sizeof(struct pkt_head)))
> +
> +#define SHMC_ALIGN_OFF_PH(v) SHMC_ALIGN(v, sizeof(struct pkt_head))
> +
> +/**
> + * Initialize a chunk.
> + * Note that the entire chunk structure lives in shared memory.
> + * @param chunk The chunk structure.
> + * @param chunksize The size of the chunk including the shm_chunk_head
> + * structure. This value is not maintained inside the chunk head
> + * but used to check alignment requirements.
> + * @return True if initialization was successful, a negative error code
> + * if initialization failed.
> + */
> +static inline int shmchunk_init(struct shm_chunk_head *chunk,
> + unsigned long chunksize)
> +{
> + if (chunk->pkg != SHMC_ALIGN_PTR_PH(chunk->pkg))
> + return -L4_EINVAL;
> + chunksize -= sizeof(struct shm_chunk_head);
> + if (chunksize != SHMC_ALIGN_OFF_PH(chunksize))
> + return -L4_EINVAL;
> + chunk->pkg[0].size = 0;
> + chunk->next_offs_to_write = 0,
> + chunk->next_offs_to_read = 0;
> + chunk->writer_blocked = 0;
> + return 0;
> +}
> +
> +/**
> + * Add a packet to the given chunk.
> + * It is the callers responsibility to ensure that it is the sender
> + * on this chunk.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @param buf The packet data.
> + * @param size The length of the packet data.
> + * @return Zero if the packet was added to the chunk, a negative error
> + * code otherwise. In particular -L4_EAGAIN means that insufficient
> + * space was available in the ring.
> + * It is the callers responsibility to wake up the receiver after adding
> + * packets or when detecting insufficient space in the buffer.
> + */
> +static inline int shmchunk_tx(struct shm_chunk_head *ch,
> + unsigned long chunksize,
> + const char *buf, unsigned long pkt_size)
> +{
> + unsigned long offset, nextoffset, r, part_len;
> + volatile struct pkt_head *ph, *next_ph;
> + int blocked = 0;
> + if (pkt_size == 0)
> + return 0;
> + if (pkt_size > chunksize - sizeof(struct pkt_head))
> + return -L4_ENOMEM;
> +
> +retry:
> + __sync_synchronize();
> + offset = ch->next_offs_to_write;
> + if (offset >= chunksize || offset != SHMC_ALIGN_OFF_PH(offset))
> + return -L4_EIO;
> + ph = ch->pkg + (offset / sizeof(struct pkt_head));
> +
> + nextoffset = SHMC_ALIGN_OFF_PH(offset + pkt_size + sizeof(struct pkt_head));
> +
> + r = ch->next_offs_to_read;
> + if (r >= chunksize)
> + return -L4_EIO;
> + if (r <= offset)
> + r += chunksize;
> +
> + /* Don't use all space, L4Linux needs an additional '0' chunk head after
> + * the chunk. Therefore we need an additional struct pkt_head.
> + */
> + if (nextoffset + sizeof(struct pkt_head) > r) {
> + /*
> + * If there is insufficient space set writer_blocked and
> + * retry. This is neccessary to avoid a race where we set
> + * writer blocked after the peer emptied the buffer. We don't
> + * set writer_blocked in the first try to avoid spurious interrupts
> + * triggered by the reader due to writer_blocked.
> + */
> + if (blocked)
> + return -L4_EAGAIN;
> + ch->writer_blocked = 1;
> + blocked = 1;
> + goto retry;
> + }
> +
> + ch->writer_blocked = 0;
> +
> + nextoffset %= chunksize;
> + /* For L4Linux compatibility */
> + next_ph = ch->pkg + (nextoffset / sizeof(struct pkt_head));
> + next_ph->size = 0;
> +
> + offset += sizeof(struct pkt_head);
> + offset %= chunksize;
> +
> + if (offset + pkt_size > chunksize)
> + part_len = chunksize - offset;
> + else
> + part_len = pkt_size;
> +
> + memcpy(((unsigned char *)ch->pkg)+offset, buf, part_len);
> + if (part_len != pkt_size) {
> + memcpy((void*)ch->pkg, buf + part_len, pkt_size - part_len);
> + }
> +
> + __sync_synchronize();
> + ph->size = pkt_size;
> + ch->next_offs_to_write = nextoffset;
> +
> + return 0;
> +}
> +
> +/**
> + * Add part of a packet to the given chunk. The data is only copied
> + * into the area of the chunk reserved for the sender. It is not made
> + * available for the receiver. Use shmchunk_tx_complete for this.
> + * It is the callers responsibility to ensure that it is the sender
> + * on this chunk.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @param buf The packet data.
> + * @param poffset The offset of this chunk within the packet. The caller must
> + * make sure that it only commits complete packets.
> + * @param len The length of the packet data.
> + * @return Zero if the packet was added to the chunk, a negative error
> + * code otherwise. In particular -L4_EAGAIN means that insufficient
> + * space was available in the ring.
> + */
> +static inline int shmchunk_tx_part(struct shm_chunk_head *ch,
> + unsigned long chunksize, const char *buf,
> + unsigned long poffset, unsigned long len)
> +{
> + unsigned long woffset, nextoffset, r, part_len, totallen;
> + volatile struct pkt_head *ph;
> + int blocked = 0;
> +
> + if (len == 0)
> + return 0;
> + if (poffset > chunksize || len > chunksize)
> + return -L4_ENOMEM;
> + totallen = poffset + len;
> + if (totallen > chunksize - 2*sizeof(struct pkt_head))
> + return -L4_ENOMEM;
> +
> +retry:
> + __sync_synchronize();
> + woffset = ch->next_offs_to_write;
> + if (woffset >= chunksize || woffset != SHMC_ALIGN_OFF_PH(woffset))
> + return -L4_EIO;
> + ph = ch->pkg + (woffset / sizeof(struct pkt_head));
> +
> + nextoffset = SHMC_ALIGN_OFF_PH(woffset + totallen
> + + sizeof(struct pkt_head));
> +
> + r = ch->next_offs_to_read;
> + if (r >= chunksize)
> + return -L4_EIO;
> + if (r <= woffset)
> + r += chunksize;
> +
> + /* Don't use all space, L4Linux needs an additional '0' chunk head after
> + * the chunk. Therefore we need an additional struct pkt_head.
> + */
> + if (nextoffset + sizeof(struct pkt_head) > r) {
> + /*
> + * If there is insufficient space set writer_blocked and
> + * retry. This is neccessary to avoid a race where we set
> + * writer blocked after the peer emptied the buffer. We don't
> + * set writer_blocked in the first try to avoid spurious interrupts
> + * triggered by the reader due to writer_blocked.
> + */
> + if (blocked)
> + return -L4_EAGAIN;
> + ch->writer_blocked = 1;
> + blocked = 1;
> + goto retry;
> + }
> +
> + ch->writer_blocked = 0;
> +
> + woffset += sizeof(struct pkt_head) + poffset;
> + woffset %= chunksize;
> +
> + if (woffset + len > chunksize)
> + part_len = chunksize - woffset;
> + else
> + part_len = len;
> +
> + memcpy(((unsigned char *)ch->pkg)+woffset, buf, part_len);
> + if (len != part_len)
> + memcpy((void*)ch->pkg, buf + part_len, len - part_len);
> +
> + return 0;
> +}
> +
> +/**
> + * Complete the transmission of a packet. This function fills in the
> + * packet head in the shm buffer. The packet data must already be present.
> + * Use shmchunk_tx_part to copy packet data.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @param pkglen The total length of the packet.
> + * @return Zero in case of success or a negative error code.
> + */
> +static inline int shmchunk_tx_complete(struct shm_chunk_head *ch,
> + unsigned long chunksize, size_t pkglen)
> +{
> + unsigned long offset, nextoffset, r;
> + volatile struct pkt_head *ph, *next_ph;
> +
> + if (pkglen == 0)
> + return 0;
> + if (pkglen > chunksize - 2*sizeof(struct pkt_head))
> + return -L4_ENOMEM;
> +
> + offset = ch->next_offs_to_write;
> + if (offset >= chunksize || offset != SHMC_ALIGN_OFF_PH(offset))
> + return -L4_EIO;
> + ph = ch->pkg + (offset / sizeof(struct pkt_head));
> +
> + nextoffset = SHMC_ALIGN_OFF_PH(offset + pkglen + sizeof(struct pkt_head));
> +
> + r = ch->next_offs_to_read;
> + if (r >= chunksize)
> + return -L4_EIO;
> + if (r <= offset)
> + r += chunksize;
> +
> + /* Don't use all space, L4Linux needs an additional '0' chunk head after
> + * the chunk. Therefore we need an additional struct pkt_head.
> + */
> + if (nextoffset + sizeof(struct pkt_head) > r)
> + return -L4_EIO;
> +
> + nextoffset %= chunksize;
> + /* For L4Linux compatibility */
> + next_ph = ch->pkg + (nextoffset / sizeof(struct pkt_head));
> + next_ph->size = 0;
> +
> + __sync_synchronize();
> +
> + ph->size = pkglen;
> + ch->next_offs_to_write = nextoffset;
> + return 0;
> +}
Isn't shmchunk_tx shmchunk_tx_part + shmchunk_tx_complete as the code
looks similar?
> +
> +/**
> + * Return the length of the next packet in the chunk.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @return Zero if the chunk is empty, the length of the first
> + * packet in the chunk or a negative value in case of an error.
> + */
> +static inline int shmchunk_rx_len(struct shm_chunk_head *ch,
> + unsigned long chunksize)
> +{
> + unsigned long offset = ch->next_offs_to_read;
> + unsigned long woffset = ch->next_offs_to_write;
> + long space = woffset - offset;
> + unsigned long pkt_size;
> + volatile struct pkt_head *ph;
> +
> + if (offset >= chunksize)
> + return -L4_EIO;
> + if (woffset >= chunksize)
> + return -L4_EIO;
> + if (space == 0)
> + return 0;
> + if (space < 0)
> + space += chunksize;
> + if (space < (int)sizeof(struct pkt_head))
> + return -L4_EIO;
> + ph = ch->pkg + (offset / sizeof(struct pkt_head));
> + pkt_size = ph->size;
> + if (pkt_size > (unsigned long)space - sizeof(struct pkt_head))
> + return -L4_EIO;
> + return pkt_size;
> +}
> +
> +/**
> + * Drop the first packet in the chunk. The packet must have non-zero length.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @return Zero if the packet could be dropped, a negative value in case
> + * of an error.
> + */
> +static inline int shmchunk_rx_drop(struct shm_chunk_head *ch,
> + unsigned long chunksize)
> +{
> + unsigned long offset = ch->next_offs_to_read;
> + unsigned long woffset = ch->next_offs_to_write;
> + long space = woffset - offset;
> + unsigned long pkt_size;
> + volatile struct pkt_head *ph;
> +
> + if (offset >= chunksize)
> + return -L4_EIO;
> + if (woffset >= chunksize)
> + return -L4_EIO;
> + if (space == 0)
> + return -L4_ENOENT;
> + if (space < 0)
> + space += chunksize;
> + if (space < (int)sizeof(struct pkt_head))
> + return -L4_EIO;
> + ph = ch->pkg + (offset / sizeof(struct pkt_head));
> + pkt_size = ph->size;
> + if (pkt_size > (unsigned long)space - sizeof(struct pkt_head))
> + return -L4_EIO;
> + offset = SHMC_ALIGN_OFF_PH(offset + sizeof(struct pkt_head) + pkt_size);
> + offset %= chunksize;
> + ch->next_offs_to_read = offset;
> + __sync_synchronize();
> +
> + return 0;
> +}
> +
> +/**
> + * Copy part of a packet from the shm chunk to a buffer. The caller
> + * must make sure that the packet contains enough data and that the
> + * buffer is long enough to receive the data.
> + * @param ch The chunk. The data in the chunk is not modified!
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @param poffset The offset of the part to copy within the packet.
> + * It is an error if this offset is beyond the length of the packet.
> + * @param len The amount to copy. It is an error if the packet is shorter
> + * than offset+len bytes.
> + * @param buf The target buffer. The caller must make sure that the buffer
> + * can hold len bytes.
> + * @return Zero or a negative error code.
> + */
> +static inline int shmchunk_rx_part(const struct shm_chunk_head *ch,
> + unsigned long chunksize,
> + unsigned long poffset,
> + unsigned long len, char *buf)
> +{
> + unsigned long roffset = ch->next_offs_to_read;
> + unsigned long woffset = ch->next_offs_to_write;
> + long space = woffset - roffset;
> + volatile const struct pkt_head *ph;
> + unsigned long part_len, pkt_size;
> +
> + if (roffset >= chunksize)
> + return -L4_EIO;
> + if (woffset >= chunksize)
> + return -L4_EIO;
> + if (space < 0)
> + space += chunksize;
> + if (space < (int)sizeof(struct pkt_head))
> + return -L4_EIO;
> + ph = ch->pkg + (roffset / sizeof(struct pkt_head));
> + pkt_size = ph->size;
> + if (pkt_size > (unsigned long)space - sizeof(struct pkt_head))
> + return -L4_EIO;
> + /* L4Linux compatibility (pkt_size == 0 means no more packets) */
> + if (pkt_size == 0)
> + return -L4_ENOENT;
> + if (poffset >= pkt_size || len > pkt_size || poffset + len > pkt_size)
> + return -L4_ENOENT;
> + roffset += poffset + sizeof(struct pkt_head);
> + roffset %= chunksize;
> +
> + if (roffset + len > chunksize)
> + part_len = chunksize - roffset;
> + else
> + part_len = len;
> + memcpy(buf, ((unsigned char *)ch->pkg)+roffset, part_len);
> + if (part_len != len)
> + memcpy(buf + part_len, (unsigned char *)ch->pkg, len - part_len);
> +
> + return 0;
> +}
> +
> +/**
> + * Remove a packet from the given chunk.
> + * It is the callers responsibility to ensure that it is the receiver
> + * on this chunk.
> + * @param ch The chunk.
> + * @param chunksize The size of the payload data in the chunk, i.e.
> + * excluding the leading shm_chunk_head structure.
> + * @param buf The packet buffer.
> + * @param size The length of the packet buffer.
> + * @return The size of the received packet, zero if no packet was available
> + * and a negative error code otherwise.
> + * It is the callers responsibility to wake up a potentially blocked
> + * sender after removing data from the buffer.
> + */
> +static inline int shmchunk_rx(struct shm_chunk_head *ch,
> + unsigned long chunksize,
> + char *buf, unsigned long buf_size)
> +{
> + unsigned long offset = ch->next_offs_to_read;
> + unsigned long woffset = ch->next_offs_to_write;
> + long space = woffset - offset;
> + volatile struct pkt_head *ph;
> + unsigned long part_len, pkt_size;
> +
> + /* It is not sufficient that we check ph->size later on, we must check that
> + * (*ph) actually contains valid data.
> + */
> + if (offset >= chunksize)
> + return -L4_EIO;
> + if (woffset >= chunksize)
> + return -L4_EIO;
> + if (space == 0)
> + return 0;
> + if (space < 0)
> + space += chunksize;
> + if (space < (int)sizeof(struct pkt_head))
> + return -L4_EIO;
> + ph = ch->pkg + (offset / sizeof(struct pkt_head));
> + pkt_size = ph->size;
> + if (pkt_size > (unsigned long)space)
> + return -L4_EIO;
> + /* L4Linux compatibility (pkt_size == 0 means no more packets) */
> + if (pkt_size == 0)
> + return 0;
> + if (buf_size < pkt_size)
> + return -L4_ENOMEM;
> + offset += sizeof(struct pkt_head);
> + offset %= chunksize;
> +
> + if (offset + pkt_size > chunksize)
> + part_len = chunksize - offset;
> + else
> + part_len = pkt_size;
> + memcpy(buf, ((unsigned char *)ch->pkg)+offset, part_len);
> + if (part_len != pkt_size)
> + memcpy(buf + part_len, (unsigned char *)ch->pkg, pkt_size - part_len);
> +
> + offset = SHMC_ALIGN_OFF_PH(offset + pkt_size);
> + offset %= chunksize;
> + ch->next_offs_to_read = offset;
> +
> + __sync_synchronize();
> + return pkt_size;
> +}
Is there are chance to use shmchunk_rx_drop in shmchunk_rx a bit?
> +/*
> + * Various wrappers for shmchunk_* functions using a single shmbuf struct
> + */
> +
> +struct shmbuf {
> + struct shm_chunk_head *tx_head;
> + struct shm_chunk_head *rx_head;
> + unsigned long tx_ring_size;
> + unsigned long rx_ring_size;
> +};
> +
> +/**
> + * Initialize an shmbuf structure with pre-allocated Rx/Tx rings
> + * @param sb A pre-allocated shmbuf structure to initialize.
> + * @param rx_chunk The receive ring for this thread.
> + * @param rx_size The size of the receive ring including the shm_chunk_head.
> + * @param tx_chunk The transmit ring for this thread.
> + * @param tx_size The size of the transmit ring including the shm_chunk_head.
> + * @return True if successful, a negative error code if initialization
> + * failed (normally due to bad alignment).
> + * This library will ensure that this thread only adds data to the tx
> + * ring and only removes data from the rx ring.
> + * It is possible to do initialization in two steps, by calling shmbuf_init()
> + * twice, once with rx_chunk == NULL and once with tx_chunk == NULL.
> + */
> +static inline int shmbuf_init(struct shmbuf *sb, char *rx_chunk, unsigned long rx_size,
> + char *tx_chunk, unsigned long tx_size)
> +{
> + if (tx_chunk) {
> + debug_printf("shmbuf_init: tx_chunk: %lu\n", tx_size);
> + if (shmchunk_init((struct shm_chunk_head *)tx_chunk, tx_size)) {
> + debug_printf("shmbuf_init: tx_chunk not aligned\n");
> + return -L4_EINVAL;
> + }
> + sb->tx_head = (struct shm_chunk_head *)tx_chunk;
> + sb->tx_ring_size = tx_size - sizeof(struct shm_chunk_head);
> + } else {
> + sb->tx_head = NULL;
> + }
> +
> + if (rx_chunk) {
> + debug_printf("shmbuf_init: rx_chunk: %lu\n", rx_size);
> + if (shmchunk_init((struct shm_chunk_head *)rx_chunk, rx_size)) {
> + debug_printf("shmbuf_init: rx_chunk not aligned\n");
> + return -L4_EINVAL;
> + }
> + sb->rx_head = (struct shm_chunk_head *)rx_chunk;
> + sb->rx_ring_size = rx_size - sizeof(struct shm_chunk_head);
> + } else if (!tx_chunk) {
> + debug_printf("shmbuf_init: ERROR: tx_chunk == rx_chunk == NULL\n");
> + return -L4_EINVAL;
> + } else {
> + sb->rx_head = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static inline int shmbuf_rx_len(struct shmbuf *sb)
> +{
> + return shmchunk_rx_len(sb->rx_head, sb->rx_ring_size);
> +}
> +
> +static inline int shmbuf_rx_drop(struct shmbuf *sb)
> +{
> + return shmchunk_rx_drop(sb->rx_head, sb->rx_ring_size);
> +}
> +
> +static inline int shmbuf_rx_part(struct shmbuf *sb, unsigned long poffset,
> + unsigned long len, char *buf)
> +{
> + return shmchunk_rx_part(sb->rx_head, sb->rx_ring_size, poffset, len, buf);
> +}
> +
> +static inline unsigned long shmbuf_tx_free(struct shmbuf *sb)
> +{
> + return shmchunk_tx_free(sb->tx_head, sb->tx_ring_size);
> +}
> +
> +static inline int shmbuf_tx(struct shmbuf *sb, const char *buf,
> + unsigned long pkt_size)
> +{
> + return shmchunk_tx(sb->tx_head, sb->tx_ring_size, buf, pkt_size);
> +}
> +
> +static inline int shmbuf_rx(struct shmbuf *sb, char *buf,
> + unsigned long buf_size)
> +{
> + return shmchunk_rx(sb->rx_head, sb->rx_ring_size, buf, buf_size);
> +}
> +
> +
> +static inline int shmbuf_tx_part(struct shmbuf *sb, const char *buf,
> + unsigned long poffset, unsigned long len)
> +{
> + return shmchunk_tx_part(sb->tx_head, sb->tx_ring_size, buf, poffset, len);
> +}
> +
> +static inline int shmbuf_tx_complete(struct shmbuf *sb, size_t pktlen)
> +{
> + return shmchunk_tx_complete(sb->tx_head, sb->tx_ring_size, pktlen);
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/src/l4/pkg/shmc/include/shmc.h b/src/l4/pkg/shmc/include/shmc.h
> index 72f224b..d90e819 100644
> --- a/src/l4/pkg/shmc/include/shmc.h
> +++ b/src/l4/pkg/shmc/include/shmc.h
> @@ -216,6 +216,20 @@ l4shmc_get_chunk_to(l4shmc_area_t *shmarea,
> l4shmc_chunk_t *chunk);
>
> /**
> + * \brief Iterate over names of all existing chunks
> + * \ingroup api_l4shmc_chunk
> + *
> + * \param shmarea Shared memory area.
> + * \param chunk_name Where the name of the current chunk will be stored
> + * \param offs 0 to start iteration, return value of previous
> + * call to l4shmc_iterate_chunk() to get next chunk
> + * \return <0 on error, 0 if no more chunks, >0 iterator value for next call
> + */
> +L4_CV long
> +l4shmc_iterate_chunk(l4shmc_area_t *shmarea, const char **chunk_name,
> + long offs);
> +
> +/**
> * \brief Attach to signal.
> * \ingroup api_l4shmc_signal
> *
> @@ -468,7 +482,7 @@ l4shmc_chunk_ptr(l4shmc_chunk_t *chunk);
> * \param chunk Chunk.
> * \return 0 on success, <0 on error
> */
> -L4_CV L4_INLINE l4_umword_t
> +L4_CV L4_INLINE long
> l4shmc_chunk_size(l4shmc_chunk_t *chunk);
>
> /**
> @@ -478,7 +492,7 @@ l4shmc_chunk_size(l4shmc_chunk_t *chunk);
> * \param chunk Chunk.
> * \return 0 on success, <0 on error
> */
> -L4_CV L4_INLINE l4_umword_t
> +L4_CV L4_INLINE long
> l4shmc_chunk_capacity(l4shmc_chunk_t *chunk);
>
> /**
> @@ -522,6 +536,37 @@ l4shmc_check_magic(l4shmc_chunk_t *chunk);
> L4_CV L4_INLINE long
> l4shmc_area_size(l4shmc_area_t *shmarea);
>
> +/**
> + * \brief Get free size of shared memory area. To get the max size to
> + * pass to l4shmc_add_chunk, substract l4shmc_chunk_overhead().
> + * \ingroup api_l4shm
> + *
> + * \param shmarea Shared memory area.
> + * \return <0 on error, otherwise: free capacity in the area.
> + *
> + */
> +L4_CV long
> +l4shmc_area_size_free(l4shmc_area_t *shmarea);
> +
> +/**
> + * \brief Get memory overhead per area that is not available for chunks
> + * \ingroup api_l4shm
> + *
> + * \return size of the overhead in bytes
> + */
> +L4_CV long
> +l4shmc_area_overhead(void);
> +
> +/**
> + * \brief Get memory overhead required in addition to the chunk capacity
> + * for adding one chunk
> + * \ingroup api_l4shm
> + *
> + * \return size of the overhead in bytes
> + */
> +L4_CV long
> +l4shmc_chunk_overhead(void);
> +
> #include <l4/shmc/internal.h>
>
> __END_DECLS
> diff --git a/src/l4/pkg/shmc/include/types.h b/src/l4/pkg/shmc/include/types.h
> index f042b53..0dcb807 100644
> --- a/src/l4/pkg/shmc/include/types.h
> +++ b/src/l4/pkg/shmc/include/types.h
> @@ -33,14 +33,17 @@ enum {
>
> /* l4shmc_chunk_desc_t is shared among address spaces */
> /* private: This data structure is hidden for the clients */
> +/* we don't make the whole struct volatile to avoid compiler warnings when
> + * using _name
> + */
> typedef struct {
> - l4_umword_t _magic; // magic
> - l4_addr_t _offset; // offset of chunk in shm-area
> - l4_umword_t _capacity; // capacity in bytes of chunk
> - l4_umword_t _size; // size of current payload
> - l4_umword_t _status; // status of chunk
> + volatile l4_umword_t _magic; // magic
> + volatile l4_addr_t _offset; // offset of chunk in shm-area
> + volatile l4_umword_t _capacity; // capacity in bytes of chunk
> + volatile l4_umword_t _size; // size of current payload
> + volatile l4_umword_t _status; // status of chunk
> char _name[L4SHMC_CHUNK_NAME_STRINGLEN]; // name of chunk
> - l4_addr_t _next; // next chunk in shm-area, as absolute offset
> + volatile l4_addr_t _next; // next chunk in shm-area, as absolute offset
No volatiles like this! Use access functions *(volatile T*)&x if you need
to read non-cached values.
> char payload[];
> } l4shmc_chunk_desc_t;
>
> @@ -50,6 +53,7 @@ typedef struct {
> l4re_ds_t _shm_ds;
> void *_local_addr;
> char _name[L4SHMC_NAME_STRINGLEN];
> + l4_umword_t _size;
> } l4shmc_area_t;
>
> /* l4shmc_signal_t is local to one address space */
> @@ -62,4 +66,5 @@ typedef struct {
> l4shmc_chunk_desc_t *_chunk;
> l4shmc_area_t *_shm;
> l4shmc_signal_t *_sig;
> + l4_umword_t _capacity;
> } l4shmc_chunk_t;
> diff --git a/src/l4/pkg/shmc/lib/src/shmc.c b/src/l4/pkg/shmc/lib/src/shmc.c
> index db713e5..5496fc7 100644
> --- a/src/l4/pkg/shmc/lib/src/shmc.c
> +++ b/src/l4/pkg/shmc/lib/src/shmc.c
> @@ -36,6 +36,7 @@ enum {
> SHMAREA_LOCK_FREE, SHMAREA_LOCK_TAKEN,
> };
>
> +#define MAX_SIZE ((~0UL) >> 1)
enum?
>
> static inline l4shmc_chunk_desc_t *
> chunk_get(l4_addr_t o, void *shm_local_addr)
> @@ -49,7 +50,10 @@ l4shmc_create(const char *shm_name, l4_umword_t shm_size)
> shared_mem_t *s;
> l4re_ds_t shm_ds = L4_INVALID_CAP;
> l4re_namespace_t shm_cap;
> - long r = -L4_ENOMEM;
> + long r;
> +
> + if (shm_size > MAX_SIZE)
> + return -L4_ENOMEM;
>
> shm_cap = l4re_get_env_cap(shm_name);
> if (l4_is_invalid_cap(shm_cap))
> @@ -66,6 +70,7 @@ l4shmc_create(const char *shm_name, l4_umword_t shm_size)
> goto out_shm_free_mem;
>
> s->_first_chunk = 0;
> + s->lock = SHMAREA_LOCK_FREE;
>
> r = l4re_ns_register_obj_srv(shm_cap, "shm", shm_ds, L4RE_NS_REGISTER_RW);
> l4re_rm_detach_unmap((l4_addr_t)s, L4RE_THIS_TASK_CAP);
> @@ -90,6 +95,7 @@ l4shmc_attach_to(const char *shm_name, l4_umword_t timeout_ms,
>
> strncpy(shmarea->_name, shm_name, sizeof(shmarea->_name));
> shmarea->_name[sizeof(shmarea->_name) - 1] = 0;
> + shmarea->_local_addr = 0;
>
> if (l4_is_invalid_cap(shmarea->_shm_ds = l4re_util_cap_alloc()))
> return -L4_ENOMEM;
> @@ -107,9 +113,15 @@ l4shmc_attach_to(const char *shm_name, l4_umword_t timeout_ms,
> goto out_free_cap;
> }
>
> - shmarea->_local_addr = 0;
> - if ((r = l4re_rm_attach(&shmarea->_local_addr,
> - l4shmc_area_size(shmarea),
> + r = l4shmc_area_size(shmarea);
> + if (r < 0)
> + {
> + r = -L4_ENOMEM;
> + goto out_free_cap;
> + }
> + shmarea->_size = r;
> +
> + if ((r = l4re_rm_attach(&shmarea->_local_addr, shmarea->_size,
> L4RE_RM_SEARCH_ADDR, shmarea->_shm_ds,
> 0, L4_PAGESHIFT)))
> goto out_free_cap;
> @@ -120,6 +132,58 @@ out_free_cap:
> return r;
> }
>
> +L4_CV long
> +l4shmc_area_overhead(void)
> +{
> + return sizeof(shared_mem_t);
> +}
> +
> +L4_CV long
> +l4shmc_chunk_overhead(void)
> +{
> + return sizeof(l4shmc_chunk_desc_t);
> +}
> +
> +static long next_chunk(l4shmc_area_t *shmarea, l4_addr_t offs)
> +{
> + shared_mem_t *shm_addr = (shared_mem_t *)shmarea->_local_addr;
> + l4shmc_chunk_desc_t *p;
> + l4_addr_t next;
> +
> + if (offs == 0)
> + {
> + next = shm_addr->_first_chunk;
> + }
> + else
> + {
> + p = chunk_get(offs, shmarea->_local_addr);
> + next = p->_next;
> + }
> + if (next == 0)
> + return 0;
> + if (next >= shmarea->_size || next + sizeof(*p) >= shmarea->_size || next <= offs)
> + return -L4_EIO;
> + if (next % sizeof(l4_addr_t) != 0)
> + return -L4_EINVAL;
> + p = chunk_get(next, shmarea->_local_addr);
> + if (p->_magic != L4SHMC_CHUNK_MAGIC)
> + return -L4_EIO;
> + return next;
> +}
> +
> +L4_CV long
> +l4shmc_iterate_chunk(l4shmc_area_t *shmarea, const char **chunk_name, long offs)
> +{
> + if (offs < 0)
> + return -L4_EINVAL;
> + offs = next_chunk(shmarea, offs);
> + if (offs > 0)
> + {
> + l4shmc_chunk_desc_t *p = chunk_get(offs, shmarea->_local_addr);
> + *chunk_name = p->_name;
> + }
> + return offs;
> +}
>
> L4_CV long
> l4shmc_add_chunk(l4shmc_area_t *shmarea,
> @@ -129,69 +193,91 @@ l4shmc_add_chunk(l4shmc_area_t *shmarea,
> {
> shared_mem_t *shm_addr = (shared_mem_t *)shmarea->_local_addr;
>
> - l4shmc_chunk_desc_t *p;
> + l4shmc_chunk_desc_t *p = NULL;
> l4shmc_chunk_desc_t *prev = NULL;
> + l4_addr_t offs = 0;
> + long ret;
>
> - shm_addr->lock = 0;
> + if (chunk_capacity >> (sizeof(chunk_capacity) * 8 - 1))
> + return -L4_ENOMEM;
>
> while (!l4util_cmpxchg(&shm_addr->lock, SHMAREA_LOCK_FREE,
> SHMAREA_LOCK_TAKEN))
> l4_sleep(1);
> asm volatile ("" : : : "memory");
> - {
> - l4_addr_t offs;
> - long shm_sz;
> - if (shm_addr->_first_chunk)
> - {
> - offs = shm_addr->_first_chunk;
> - p = chunk_get(offs, shmarea->_local_addr);
> - do
> - {
> - offs = p->_offset + p->_capacity + sizeof(*p);
> - prev = p;
> - p = chunk_get(p->_next, shmarea->_local_addr);
> - }
> - while (prev->_next);
> - }
> - else
> - // first chunk starts right after shm-header
> - offs = sizeof(shared_mem_t);
> -
> - if ((shm_sz = l4shmc_area_size(shmarea)) < 0)
> - goto out_free_lock;
> -
> - if (offs + chunk_capacity + sizeof(*p) >= (unsigned long)shm_sz)
> - goto out_free_lock; // no more free memory in this shm
> -
> - p = chunk_get(offs, shmarea->_local_addr);
> - p->_offset = offs;
> - p->_next = 0;
> - p->_capacity = chunk_capacity;
> - // Ensure that other CPUs have correct data before inserting chunk
> - __sync_synchronize();
> -
> - if (prev)
> - prev->_next = offs;
> - else
> - shm_addr->_first_chunk = offs;
> - }
> - __sync_synchronize();
> - shm_addr->lock = SHMAREA_LOCK_FREE;
> + while ((ret = next_chunk(shmarea, offs)) > 0)
> + {
> + p = chunk_get(ret, shmarea->_local_addr);
> + if (strcmp(p->_name, chunk_name) == 0)
> + {
> + ret = -L4_EEXIST;
> + goto out_free_lock;
> + }
> + offs = ret;
> + }
> + if (ret < 0)
> + goto out_free_lock;
> + if (offs == 0)
> + offs = sizeof(shared_mem_t);
> + else
> + {
> + l4_addr_t n = p->_offset + p->_capacity + sizeof(*p);
> + if (n <= offs || n >= shmarea->_size)
> + {
> + ret = -L4_EIO;
> + goto out_free_lock;
> + }
> + offs = n;
> + prev = p;
> + }
>
> + if (offs + chunk_capacity + sizeof(*p) > (unsigned long)shmarea->_size)
> + {
> + ret = -L4_ENOMEM;
> + goto out_free_lock; // no more free memory in this shm
> + }
> + p = chunk_get(offs, shmarea->_local_addr);
> + p->_offset = offs;
> + p->_next = 0;
> + p->_capacity = chunk_capacity;
> p->_size = 0;
> p->_status = L4SHMC_CHUNK_CLEAR;
> p->_magic = L4SHMC_CHUNK_MAGIC;
> strncpy(p->_name, chunk_name, sizeof(p->_name));
> p->_name[sizeof(p->_name) - 1] = 0;
> + // Ensure that other CPUs have correct data before inserting chunk
> + __sync_synchronize();
> +
> + if (prev)
> + prev->_next = offs;
> + else
> + shm_addr->_first_chunk = offs;
> +
> + __sync_synchronize();
> + shm_addr->lock = SHMAREA_LOCK_FREE;
>
> - chunk->_chunk = p;
> - chunk->_shm = shmarea;
> - chunk->_sig = NULL;
> + chunk->_chunk = p;
> + chunk->_shm = shmarea;
> + chunk->_sig = NULL;
> + chunk->_capacity = chunk_capacity;
>
> return L4_EOK;
> out_free_lock:
> shm_addr->lock = SHMAREA_LOCK_FREE;
> - return -L4_ENOMEM;
> + return ret;
> +}
> +
> +L4_CV long
> +l4shmc_area_size_free(l4shmc_area_t *shmarea)
> +{
> + long ret;
> + l4_addr_t offs = 0;
> + while ((ret = next_chunk(shmarea, offs)) > 0)
> + offs = ret;
> + if (ret < 0)
> + return ret;
> + ret = shmarea->_size - offs;
> + return ret > 0 ? ret : 0;
> }
>
> L4_CV long
> @@ -216,13 +302,11 @@ l4shmc_add_signal(l4shmc_area_t *shmarea,
> b[sizeof(b) - 1] = 0;
>
> tmp = l4re_get_env_cap(shmarea->_name);
> + r = -L4_ENOENT;
> if (l4_is_invalid_cap(tmp))
> - {
> - r = -L4_ENOENT;
> - goto out_free_sigcap;
> - }
> + goto out_free_sigcap;
>
> - if ((r = l4re_ns_register_obj_srv(tmp, b, signal->_sigcap, 0)) != L4_EOK)
> + if (l4re_ns_register_obj_srv(tmp, b, signal->_sigcap, 0))
> goto out_free_sigcap;
>
> return L4_EOK;
> @@ -238,25 +322,30 @@ l4shmc_get_chunk_to(l4shmc_area_t *shmarea,
> l4shmc_chunk_t *chunk)
> {
> l4_kernel_clock_t try_until = l4re_kip()->clock + (timeout_ms * 1000);
> - shared_mem_t *shm_addr = (shared_mem_t *)shmarea->_local_addr;
> + long ret;
>
> do
> {
> - l4_addr_t offs = shm_addr->_first_chunk;
> - while (offs)
> + l4_addr_t offs = 0;
> + while ((ret = next_chunk(shmarea, offs)) > 0)
> {
> l4shmc_chunk_desc_t *p;
> + offs = ret;
> p = chunk_get(offs, shmarea->_local_addr);
> -
> if (!strcmp(p->_name, chunk_name))
> { // found it!
> - chunk->_shm = shmarea;
> - chunk->_chunk = p;
> - chunk->_sig = NULL;
> + chunk->_shm = shmarea;
> + chunk->_chunk = p;
> + chunk->_sig = NULL;
> + chunk->_capacity = p->_capacity;
> + if (chunk->_capacity > shmarea->_size ||
> + chunk->_capacity + offs > shmarea->_size)
> + return -L4_EIO;
> return L4_EOK;
> }
> - offs = p->_next;
> }
> + if (ret < 0)
> + return ret;
>
> if (!timeout_ms)
> break;
Adam
--
Adam adam at os.inf.tu-dresden.de
Lackorzynski http://os.inf.tu-dresden.de/~adam/
More information about the l4-hackers
mailing list