Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netbuf: add initial support #12315

Closed
wants to merge 1 commit into from
Closed

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Sep 27, 2019

Contribution description

This PR adds an abstraction for allocating packets from a network stack or static buffer.
The motivation behind this is:

  • To keep zero copy, the network stack is the one who in practice allocates the buffer for the packet.
    At the same time if we want to have MAC and PHY layers that are independent to the network stack, we need an interface for allocation.
  • We can "instruct" the lower layers to allocate packets so the PHY layer can allocate a packet just right after reading the pkt_len (e.g first byte of the IEEE802.15.4 PHY frame). So we could think of dropping the "get packet lengh" and "drop packet" behavior of dev->driver->recv (see [RFC] netdev: change receive mechanism #11652)

If CONFIG_NETBUF_DISABLE_STACK_ALLOC==0, it's necessary to define a netbuf_stack_alloc function for every stack.

How to use

Let's assume there's a radio API for reading data from the framebuffer:

int radio_read_packet(radio_t radio, netbuf_t *netbuf)
{
    char pkt_len;
    radio_read_framebuffer(radio, &pkt_len, 1);

    /* Allocate pkt of "pkt_len" bytes long */
    void *pkt = netbuf_alloc(&netbuf, pkt_len);
    if(!pkt) {
        return -ENOBUFS;
    }
    radio_read_framebuffer(radio, pkt, pkt_len);
    return pkt_len;
}

Depending on how the netbuf is initialized, it's possible to either get a buffer from the stack allocator or from a static buffer.
Let's assume GNRC has the following netbuf_stack_alloc function:

void *netbuf_stack_alloc(netbuf_t *netbuf, size_t size)
{
    gnrc_pktsnip_t *pkt = gnrc_pktbuf_add(NULL, NULL, size, GNRC_NETTYPE_UNDEF);
    if(!pkt) {
        return NULL;
    }
    netbuf->ctx = pkt;
    netbuf->data = pkt->data;
    netbuf->size = size;
    return pkt->data;
}

Some examples:

  • Dynamically allocate packet when receiving:
netbuf_t netbuf;
netbuf_init(&netbuf, NULL, 0); /* Pass NULL to ask the stack to allocate a packet when calling netbuf_alloc */
radio_read_packet(radio, &netbuf);
/* if everything is OK, netbuf.data contains the packet of length netbuf.size. */
/* If using GNRC, netbuf.ctx has a pointer to the pkt snip so it can be released afterwards */
  • Pass a fixed buffer (useful when the size of the packet is known, e.g IEEE802.15.4 ACK packet)
netbuf_t netbuf;
char ack[5]; /* Allocate space for the IEEE802.15.4 ACK */
netbuf_init(&netbuf, ack, 5); /* Pass the ACK buffer to the netbuf */
radio_read_packet(radio, &netbuf);
/*The ACK should be in the ack buffer*/

Considerations about the RX procedure

I wrote this patch that overloads the "recv" function of netdev to receive a netbuf instead of a buffer, and removed the "drop" and "get packet length" logic.

The "recv" function will read the pkt_len, try to get a buffer from the netbuf and write directly to that buffer. The frame buffer is read once and the logic is simpler now.

It also makes the binary smaller from:

   text    data     bss     dec     hex filename
  42864     508    6232   49604    c1c4 /home/jialamos/Development/RIOT/examples/default/bin/iotlab-m3/default.elf

to

   text    data     bss     dec     hex filename
  42716     508    6232   49456    c130 /home/jialamos/Development/RIOT/examples/default/bin/iotlab-m3/default.elf

And calling netif->ops->recv() is 6-8% faster with this method.

Testing procedure

  • Check the documentation is OK with make doc
  • Try the proposed patch.
  • TBD: I will add some tests soon.

Issues/PRs references

It's related to the PHY/MAC rework proposed in #11483
#11652

@jia200x jia200x added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Sep 27, 2019
@miri64
Copy link
Member

miri64 commented Sep 27, 2019

Isn't there a free function missing?

@jia200x
Copy link
Member Author

jia200x commented Sep 27, 2019

Isn't there a free function missing?

Maybe at some point, but usually the "netbuf" object will end up in stack functions, which already know how to release the packet.

@miri64
Copy link
Member

miri64 commented Sep 27, 2019

Maybe at some point, but usually the "netbuf" object will end up in stack functions, which already know how to release the packet.

So when sending a packet this will not be used?

@jia200x
Copy link
Member Author

jia200x commented Sep 27, 2019

So when sending a packet this will not be used?

I'm not sure yet. The upper layer will usually pass an iolist like object, which doesn't look like the netbuf structure. There will be indeed a function for releasing resources, but not sure yet if belongs to this netbuf API.

@jia200x
Copy link
Member Author

jia200x commented Sep 27, 2019

(If we need to add a free mechanism it could be done in a follow up)

@jia200x
Copy link
Member Author

jia200x commented Nov 11, 2019

The definition of #12691 for netbuf_t is different. However, in this PR there are still some functionalities that can be added there. So, I will keep this open.

@jia200x
Copy link
Member Author

jia200x commented Mar 31, 2020

As described in #12691 (comment), this is outdated now. I will close it

@jia200x jia200x closed this Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants