21 Feb
2014
21 Feb
'14
10:12 a.m.
Hello Andreas
On 15.02.2014 10:25, Andreas Eversberg wrote:
So it can be removed completely?
What about putting such definitions somewhere central (e.g. wiki or top
of msgb.h)?
Jacob Erlbeck wrote:
The line
uint8_t l3_len = msg->tail - msg->data;
delivers the correct result, that's not the point. But here are 2
different things being mixed:
- the l3 length definition: l3_len = msg->tail - msg->l3h (AKA
msgb_l3len())
- the assumption that msg->data == msg->l3h
Mixing things this way is IMO conceptually broken and hard to maintain.
According to your statement below ("frames that are sent to lower layer
1 should have l2h and l3h set [...]"), l3h must have been set when
lapdm_send_ph_data_req() is entered. The requirement data == l3h on the
other hand is more than requested in general, and it would be helpful if
it was documented and/or asserted.
- L3 length computation should be done with the
macros, how it is done
in lapdm_send_ph_data_req() is broken.
what is actually broken? i see that l3_len
is the complete msgb space
between data and tail. - Why does
lapd_msg_ctx have a length field that is not used?
i guess it was used in early
code (when osmocombb project started), but
it is actually not required. > - How
l2/data/.. in a msg are expected to be used/set should be
> documented somewhere.
each msg has l1h, l2h and l3h pointers. the logical way
is (even if it
is currently not implemented correctly):
frames that are received from lower layer 1 should have l2h set and may
have l1 header or not. (layer 2 is not responsible of l1 header, so we
should ignore it.) l3h must not be set, since layer 1 does not know
about any l3 header start. the l2_ph_data_ind() function should parse
the l2h, set l3h and remove everything up to l3h.
frames that are sent to lower layer 1 should have l2h and l3h set before
sending to layer 1. see lapdm_send_ph_data_req(). push the dummy_l1 header after pushing RLL header:
msg = msgb_from_array(dummy1, sizeof(dummy1));
rsl_rll_push_l3(msg, RSL_MT_DATA_REQ, 0, 0, 1);
+ msgb_push(msg, dummy_l1len);
- first the msg is allocated and filled with array. l3h is set.
- rsl_rll_push_l3 pushes the layer 3 information element and the RLL
header in front of the msg and sets l2h.
- at least msgb_push adds the dummy_l1 header in front of everything.
l1h is not set, but it is not relevant, since l1 header is ignored by
lapdm.
Thank you, that makes sense.
Jacob