This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Harald Welte laforge at gnumonks.orgHi Neels and Pablo,
first of all, it is great to see this code appear, I never liked libgtp
much either...
On Thu, Oct 15, 2015 at 03:32:02PM +0200, Neels Hofmeyr wrote:
> > +struct gtp0_header {
> > +#if BYTE_ORDER == BIG_ENDIAN
> > + uint8_t version:3,
> > + pt:1,
> > + spare:3,
> > + snn:1;
> > +#elif BYTE_ORDER == LITTLE_ENDIAN
> > + uint8_t snn:1,
> > + spare:3,
> > + pt:1,
> > + version:3;
>
> Hmm, endianness is about *byte* order, not *bit* order, right?
Life would be too simple for that. It there is both byte-endinanness
and bit-endianness. And if you define bit-fields using the c syntax,
then (at least on all platforms I know), you have to split your
definition like above. Look at the definitions of IP header and TCP
header in your /usr/include/netinet/ip.h or /usr/include/netinet/tcp.h
> to use one of the existing decoding functions for endianness instead:
> - ntohs()/ntohl()
that doesn't help you with what pablo defined, as it is about an uint8_t ;)
> > + gtp0h->seq = htons(seq);
> > + gtp0h->tid = htobe64(tid);
>
> Ah, here it is: htons() stored in the header struct.
> I'd prefer a host-byte-order struct.
The struct itself (for 16bit/32bit members of the struct) dosen't care.
And the function you refer to takes host-byte-order input and then
pushes the values as network byte ordre to the msgb, which I think is
fine.
> > +void gtp0_header_end(struct gtp0_header *gtp0h, struct msgb *msg)
> > +{
> > + gtp0h->length = htons(msg->len - sizeof(*gtp0h));
>
> With msgb_alloc_headroom(), it is possible to first write the IEs to the
> msgb, and then prepend the header with the correct size later on. See for
> example in openbsc, gprs_gsup_msgb_alloc() and ipa_msg_push_header().
yes, that is the preferred method.
> > +static const struct tlv_definition gtp_pdp_create_req_attr_tlvdef = {
> > + .def = {
> > + [GTPV0_IE_QOS_PROFILE] = { TLV_TYPE_FIXED, 3 },
> > +};
>
> I can't begin to express how much better this looks than the old gtpie.h!
Well, that's how we generaly deal with TLVs in libosmocore based
programs :)
> > +void *msgb_tv_put_be24(struct msgb *msg, int type, uint32_t value)
> > +{
> > + value = htonl(value & 0x00ffffff) >> 8;
>
> heh, interesting :)
> This won't work on a BE system. You can't portably do a
> host CPU shift-right on a network byte order value.
On a BE system the htonl() will evaporate and you end up with doning
only a shift-right by 8, which is probably not what you wanted, indeed.
> Imagining a class: Big Endian 101, Professor asks: "Can anyone explain
> the results of this code on a big endian and a little endian system?"
> Half an hour of vigorous discussion follows.
Beware, Pablo is a CS Professor at Sevilla University ;)
--
- Harald Welte <laforge at gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)