Attention is currently required from: jolly, neels, laforge, pespin, fixeria.
7 comments:
Commit Message:
Patch Set #8, Line 16: are accepted.
Agree, I also mentioned that in my last review.
In l1sap_rtp_rx_cb() we convert the payload but the conversion is only carried out when there is an exact length match. If there are packets with invalid payloads (either too long or to short) received they would slip through. This code path is not about gatekeeping, its only about the conversion.
in rtppayload_is_valid() we make sure that the payload length matches the expectations of the BTS and ensures that no unsupported payload is passed on to lower layers.
That are essentially two different topics and I think it makes sense to split those in separate patches. I have spitted the conversion part in a separate patch.
File src/common/bts.c:
add "...is supported" or "support for ... […]
Done
same
Done
File src/common/l1sap.c:
Patch Set #7, Line 1274: if (OSMO_UNLIKELY(rfc5993 && (resp_msg->len != GSM_HR_BYTES_RTP_RFC5993))) {
I agree that a flag being zero should disallow that format, no matter what the other flag's value is […]
I think this is fixed now. In case no format is set both payload formats would be rejected. So a the hardware specific code now must have the flags set accordingly.
File src/common/l1sap.c:
Patch Set #8, Line 1274: /* BTS flags specify that RFC 5993 (and not ETSI TS 101.318) is understood by lower layers */
Ack
Done
Patch Set #8, Line 1952: * formats are supported (either by setting both or none of the flags), no conversion will be carried out. */
Ack
I have splatted the converting part into a follow up patch - so it is rejecting
Patch Set #8, Line 1969: break;
the code converting from one format to the other should be in separate functions
This turns out even more ugly due to the l1sap_msgb_alloc and the fallthrough.
Anyway, lets discuss this in: I9419b40c1171876879d41aba4f51c93e8ef5673c then
To view, visit change 31417. To unsubscribe, or for help writing mail filters, visit settings.