Hi,
I see that while adding decompression support dexter has fixed a bug in the sndcp code. Before the GTP-U frame would include the LLC FCS and now we do this:
npdu_len = (msg->data + msg->len) - npdu - 3;
What I wonder (and the commit message didn't say is) was the fragment part handled as well. The calculation is:
/* make sure to subtract length of SNDCP header from 'len' */ rc = defrag_enqueue(sne, suh->seg_nr, data, len - (data - hdr));
and len is passed in from the TLV struct in gprs_llc_rcvmsg. I think the fragmented case is working okay.
Shall we try to unify the length calculation to rely on the local "len" variable instead of checking msg->data+msg->len?
cheers holger
On 9. May 2017, at 14:12, Holger Freyther holger@freyther.de wrote:
npdu_len = (msg->data + msg->len) - npdu - 3;
What I wonder (and the commit message didn't say is) was the fragment part handled as well. The calculation is:
/* make sure to subtract length of SNDCP header from 'len' */ rc = defrag_enqueue(sne, suh->seg_nr, data, len - (data - hdr));and len is passed in from the TLV struct in gprs_llc_rcvmsg. I think the fragmented case is working okay.
Shall we try to unify the length calculation to rely on the local "len" variable instead of checking msg->data+msg->len?
Or to make it a bit more clear. The -3 assumes that something is beyond the LLC IE. I think instead of assuming that we should use the IE len (instead of msg->len). We already do that in the fragmented case.
holger
Hi Holger,
On Wed, May 10, 2017 at 04:03:34PM +0200, Holger Freyther wrote:
Or to make it a bit more clear. The -3 assumes that something is beyond the LLC IE. I think instead of assuming that we should use the IE len (instead of msg->len). We already do that in the fragmented case.
seems very reasonable to me.
osmocom-net-gprs@lists.osmocom.org
-
Harald Welte -
Holger Freyther -
Neels Hofmeyr