Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/43575?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed: Verified+1 by Jenkins Builder
Change subject: core/socket: sockaddr_cmp: compare fields ......................................................................
core/socket: sockaddr_cmp: compare fields
osmo_sockaddr_cmp() decides whether two addresses are equal with a memcmp() over the whole struct sockaddr_in or sockaddr_in6. On Linux those structures hold only family, port, address and padding, so the comparison is exact. On Darwin and the BSDs the first byte is sin_len / sin6_len. The kernel fills it in on recvfrom() and accept(), while an address the application built from configuration leaves it zero. The same peer then compares as two different addresses.
Every static NS-VC in gprs_ns2 breaks on this. The answer to the first NS-RESET arrives from the configured remote, gprs_ns2_udp.c cannot match it to the NS-VC ("Ignoring NS RESET ACK from newconnection for non-existing NS-VC", gprs_ns2.c:1066) and the link never leaves RESET. Observed with osmo-pcu against osmo-sgsn on Darwin loopback: the SGSN side, which learned the peer from the packet, goes to BLOCKED and then loses every NS-ALIVE-ACK, while the PCU side stays in RESET. Dynamic NS-VCs are unaffected because their remote address is a copy of what recvfrom() returned.
Compare the fields instead of the bytes: port and address for AF_INET, port, flow info, address and scope id for AF_INET6, in the order memcmp() visited them, so the ordering the function gives to sorted users does not change. The default branch keeps its memcmp() over the full osmo_sockaddr. Linux behaviour is unchanged.
Add tests/sockaddr_cmp covering the two cases that regressed and the ordering the fix has to preserve: an IPv4 and an IPv6 pair equal in port and address but differing in the length byte, both expected to compare equal, and ordering by port, by address, by flow info and by scope id, each checked in both directions. Against the previous implementation the two length-byte cases return -16 and -28 instead of 0.
The length byte only exists where the sockaddr carries one, so configure.ac gains an AC_CHECK_MEMBER for struct sockaddr_in.sin_len, in the style of the existing s6_addr32 check. Where the member is absent the two addresses are identical and the expectation is the same, so the test and its .ok output are the same on every platform.
Note that sin6_flowinfo and sin6_scope_id are compared byte by byte, which is not numeric order on a little endian host. That is unchanged from the memcmp() this replaces. The test uses values differing in one byte only, where both orders agree.
Change-Id: I75fc62a92f546a906dbb1440984d62f962579255 Signed-off-by: Andrei Gosman andrei.gosman@gmail.com --- M configure.ac M src/core/socket.c M tests/Makefile.am A tests/sockaddr_cmp/sockaddr_cmp_test.c A tests/sockaddr_cmp/sockaddr_cmp_test.ok M tests/testsuite.at 6 files changed, 201 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/75/43575/3