[OpenWrt-Devel] [PATCH v2] ubus: use network order in ubus message header fields

[Eyal Birger]

Hi Felix,

Thanks for your review!

If I understood the code correctly, the header fields are never accessed
before the header is fully received - at which point the conversion is done.
My motivation for changing the fields only as they are sent on the wire is
to avoid renaming the fields or changing their semantics to avoid
backporting messes...

Could you elaborate on the corner cases? are there any tests I can run to
verify them?

Thanks again!
Eyal.


On Mon, Feb 15, 2016 at 12:05 PM Felix Fietkau <nbd at openwrt.org> wrote:

> On 2016-02-15 05:09, Eyal Birger wrote:
> > Changing the ubus message header fields from 'host' order to 'network'
> order
> > allows passing ubus messages between hosts with different endianity.
> >
> > Example use (creating a ubus proxy):
> >
> > on host A (e.g. big endian router already running ubusd), run:
> > $ socat TCP-LISTEN:5699,fork UNIX:/var/run/ubus.sock &
> >
> > On host B (e.g. little endian development PC) run:
> > $ socat UNIX-LISTEN:/var/run/ubus.sock,fork TCP:<host A IP>:5699 &
> >
> > Now ubus applications can be run on host B and seamlessly interact with
> ubus
> > applications on host A.
> >
> > Signed-off-by: Eyal Birger <eyal.birger at gmail.com>
> >
> > ----
> > v2:
> >       Applied change to ubusd too. Thanks Felix!
> > ---
> >  libubus-io.c |  7 +++++--
> >  ubusd.c      | 14 ++++++++++++--
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/ubusd.c b/ubusd.c
> > index f1f8ac7..7279a70 100644
> > --- a/ubusd.c
> > +++ b/ubusd.c
> > @@ -110,8 +110,15 @@ static int ubus_msg_writev(int fd, struct
> ubus_msg_buf *ub, int offset)
> >       }
> >
> >       if (offset < sizeof(ub->hdr)) {
> > -             iov[0].iov_base = ((char *) &ub->hdr) + offset;
> > -             iov[0].iov_len = sizeof(ub->hdr) - offset;
> > +             struct ubus_msghdr hdr;
> > +
> > +             hdr.version = ub->hdr.version;
> > +             hdr.type = ub->hdr.type;
> > +             hdr.seq = cpu_to_be16(ub->hdr.seq);
> > +             hdr.peer = cpu_to_be32(ub->hdr.peer);
> > +
> > +             iov[0].iov_base = ((char *) &hdr) + offset;
> > +             iov[0].iov_len = sizeof(hdr) - offset;
> >               iov[1].iov_base = (char *) ub->data;
> >               iov[1].iov_len = ub->len;
> >
> This has some broken corner cases, because reads can be partial. I'd
> prefer endian conversion on access instead of on receive/send, even
> though it touches a few more lines.
>
> - Felix
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20160215/5a3e5b5f/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel