Re: Receiving Network Packets with Mirage/kFreeBSD

[Anil Madhavapeddy <anil@xxxxxxxxxx>]

On Mon, Aug 20, 2012 at 11:59:04PM +0200, PALI Gabor Janos wrote:
> Following up to my original mail in order to gently motivate Robert and Anil
> to answer :-)

My (considerable) excitement about progress here has been hit by a
day-long email backlog truck, so apologies for that :-)

> On Thu, Aug 16, 2012 at 03:13:27PM +0200, PALI Gabor Janos wrote:
> > - Shared ring buffers are created as Io_pages by allocating page-aligned,
> >   contiguous, multi-page memory areas via FreeBSD's contigmalloc(9).  These
> >   are directly accessible in Mirage as character arrays.
> 
> Io_page.get currently returns a single page with the given size (in bytes),
> and not an array of pages.  I did not see the reason to do so, as one can
> always get the corresponding pages with the Io_page.to_pages function.
> 
> 
> > - Each packet arriving on any of the plugged interfaces is placed to the 
> > next
> >   available slot of the corresponding shared ring buffer with m_copydata().
> 
> I agree that m_copydata() involves a possibly unnecessary copying.  But it 
> also
> contributes to flattening the packets so Mirage can work with them without too
> much trickery.  I suppose intercepted packets should be turned into linear 
> memory
> buffers at some point.

This conveniently addresses both Robert and Mort's points.  I think that the
right thing to do is to extend the bigarray bindings to have more custom
types.  Right now, we have:

static void caml_ba_finalize(value v)
{
  struct caml_ba_array * b = Caml_ba_array_val(v);
#ifdef _KERNEL
  int data_size, i;
#endif

  switch (b->flags & CAML_BA_MANAGED_MASK) {
  case CAML_BA_EXTERNAL:
    break;
  case CAML_BA_MANAGED:
    if (b->proxy == NULL) {
#ifdef _KERNEL
      data_size = 0;
      for (i = 0; i < b->num_dims; i++)
          data_size += b->dim[i];
      contigfree(b->data, data_size, M_MIRAGE);
#else
      free(b->data);
#endif
    } else {

How about adding new CAML_BA_MANAGED flags specifically for mbufs and disk
buffers, so that we can increment the reference count and avoid copying
it?  Then, the bigarray pointer can look directly into the mbuf without a
copy, and decrement the reference using the relevant function rather than
using contigfree directly.

I think it's still useful to keep the existing contigmalloc code for other
future uses that need page-aligned code, but eliminating the mbuf data
copy is very important for performance.  It would also be good to see what
the disk buffer code looks like to make sure we can also write filesystems
in Mirage (which is an interest of Jon Anderson's, among others!)

-- 
Anil Madhavapeddy                                 http://anil.recoil.org