I screwed up the v6 tests a little, and just pushed some fixes to it. The current status is: the 4 tests that are in V5-7-patches all pass (trap2sink, v6 trap2sink, trapsess, v6 trapsess). I applied the same clientaddr port zeroing to v6 as v4 already has in V5-7-patches to get the v6 trapsink version to pass.
Since the code is all different in 5.8, the fixes don't apply. The v4 trapsink case and all of the v6 cases (including the added case for "-s" with trapsess) fail. The two v4 trapsess cases succeed. All of the failures in 5.8 are regressions from V5-7-patches. (They were regressions before your changes here, but your changes broke my fix that Sam tried in https://sourceforge.net/p/net-snmp/bugs/2888/ ) Bill On Tue, Nov 6, 2018 at 3:31 PM Bill Fenner <fen...@users.sourceforge.net> wrote: > Playing with this in V5-8-patches, I see it broke my fix for using > clientaddr to specify the source address for traps: > > netsnmp_sockaddr_in: addr 0xffd5e824, inpeername "127.0.0.1", > default_target ":0" > netsnmp_sockaddr_in: addr 0xffd5e824, inpeername ":0", default_target > "[NIL]" > netsnmp_sockaddr_in: return { AF_INET, 0.0.0.0:161 } > netsnmp_sockaddr_in: hostname (resolved okay) > netsnmp_sockaddr_in: return { AF_INET, 127.0.0.1:161 } > > In the previous implementation, specifying a default_target of ":0" would > set the port number to zero, meaning, let the kernel decide. Now, it > appears to mean, use the SNMP default. This causes the bind() to fail, of > course, since binding to 127.0.0.1:0 would work fine, but binding to > 127.0.0.1:161 conflicts with the actual snmpd port. > > netsnmp_udpbase: open remote UDP: [172.25.24.62]:5716->[0.0.0.0]:0 > netsnmp_udpbase: binding socket: 8 to UDP: [0.0.0.0]:0->[127.0.0.1]:161 > netsnmp_udpbase: failed to bind for clientaddr: 13 Permission denied > snmpd: netsnmp_create_notification_session: > /tmp/snmp-test-T180trap2sinkclientaddr_simple-7030/snmpd.conf: line 8: > Error: cannot create sink: udp:172.25.24.62:5716 > > I've committed my tests for trap2sink, trapsess, and using "-s" with > trapsess. None of the v6 versions work, but the v4 trapsess versions work, > after fixing netsnmp_udpipv4base_transport_with_source() to use the > "bind_addr" variable. I believe that fixing the parsing of ":0" to return > a zero port like it did before would fix trap2sink. I haven't looked at > what's wrong with v6 yet. > > (The confusing code in netsnmp_udpipv4base_transport() around "have_port" > and "uses_port", including the introduction > of NETSNMP_DS_LIB_CLIENT_ADDR_USES_PORT, is probably completely working > around this same bug of setting the port when it doesn't need to be set, > and could be eliminated altogether if that bug is fixed.) > > Bill > > > On Tue, Nov 6, 2018 at 11:03 AM Bill Fenner <fen...@users.sourceforge.net> > wrote: > >> Hi Bart, >> >> My main question is, what's the advantage of storing the IPv4/IPv6 >> address as a string and a port number, instead of as a sockaddr_*? I.e., >> why use netsnmp_ep_str? >> >> Is the API change here ok? Are we assuming that nobody ever calls >> netsnmp_foo_transport() directly? >> >> Should there be an #ifdef in patch 5 to make the interface name optional, >> both in the struct and the parser, for platforms that don't support it? Is >> it better to return a parse error on the address that includes interface >> name, or a "failed to open socket" without much more information? >> >> Given your proposed code structure, I imagine that we could add network >> namespaces to netsnmp_ep too - this basically ends up using "socketat( /* >> magic */, family, type, protocol )" instead of socket() to create the >> socket, and the magic can be derived from what we store in ep. >> >> I don't have a strong opinion about which this code should go in. >> >> Bill >> >> >> On Sun, Oct 28, 2018 at 5:19 PM Bart Van Assche <bvanass...@acm.org> >> wrote: >> >>> Hello, >>> >>> As you may have noticed multiple people asked to add SO_BINDTODEVICE >>> support >>> to Net-SNMP. This patch series adds such support by allowing to specify >>> the >>> name of the network interface to bind a Net-SNMP endpoint to as >>> "@<interface name>". An example: >>> >>> agent/snmpd -Mmibs -f -Lo -c .../snmpd.conf udp6:localhost@lo:1161 -r >>> apps/snmpwalk -Mmibs -v2c -cpublic udp6:localhost:1161 .1 >>> >>> The reason I'm posting this patch series on the Net-SNMP mailing list is >>> to >>> gather feedback about this patch series. Does everyone agree with the >>> code >>> changes in this patch series? If so, on which branch(es) should these >>> patches >>> be applied? Master only or v5.8 and master? >>> >>> See also: >>> * Add SO_BINDTODEVICE support >>> (https://sourceforge.net/p/net-snmp/patches/1296/). >>> * Add Linux VRF support ( >>> https://sourceforge.net/p/net-snmp/patches/1376/). >>> >>> Thanks, >>> >>> Bart. >>> >>> Bart Van Assche (6): >>> libsnmp/transports: Introduce netsnmp_parse_ep_str() >>> libsnmp/transports: Introduce netsnmp_sockaddr_in3() and >>> netsnmp_sockaddr_in6_3() >>> libsnmp/transports: Change multiple sockaddr_in* arguments into >>> netsnmp_ep >>> configure: Add a test for SO_BINDTODEVICE >>> libsnmp/transports: Add support for interface binding >>> testing/fulltests/unit-tests: Add netsnmp_parse_ep_str() unit test >>> >>> configure | 34 +++ >>> configure.d/config_os_misc4 | 15 ++ >>> include/net-snmp/library/snmpDTLSUDPDomain.h | 4 +- >>> include/net-snmp/library/snmpIPBaseDomain.h | 36 +++ >>> include/net-snmp/library/snmpIPv4BaseDomain.h | 7 + >>> include/net-snmp/library/snmpIPv6BaseDomain.h | 5 + >>> include/net-snmp/library/snmpTCPDomain.h | 4 +- >>> include/net-snmp/library/snmpTCPIPv6Domain.h | 4 +- >>> include/net-snmp/library/snmpUDPDomain.h | 8 +- >>> .../net-snmp/library/snmpUDPIPv4BaseDomain.h | 12 +- >>> include/net-snmp/library/snmpUDPIPv6Domain.h | 11 +- >>> .../net-snmp/library/snmpUDPsharedDomain.h | 6 +- >>> include/net-snmp/net-snmp-config.h.in | 3 + >>> snmplib/transports/snmpDTLSUDPDomain.c | 36 +-- >>> snmplib/transports/snmpIPBaseDomain.c | 114 +++++++++ >>> snmplib/transports/snmpIPv4BaseDomain.c | 118 +++------ >>> snmplib/transports/snmpIPv6BaseDomain.c | 238 ++++-------------- >>> snmplib/transports/snmpTCPDomain.c | 24 +- >>> snmplib/transports/snmpTCPIPv6Domain.c | 23 +- >>> snmplib/transports/snmpUDPDomain.c | 22 +- >>> snmplib/transports/snmpUDPIPv4BaseDomain.c | 58 +++-- >>> snmplib/transports/snmpUDPIPv6Domain.c | 70 +++--- >>> snmplib/transports/snmpUDPsharedDomain.c | 90 ++++--- >>> testing/fulltests/support/clib_build | 1 + >>> .../T022netsnmp_parse_ep_str_clib.c | 55 ++++ >>> win32/libsnmp/Makefile.in | 1 + >>> win32/libsnmp_dll/Makefile.in | 1 + >>> 27 files changed, 561 insertions(+), 439 deletions(-) >>> create mode 100644 include/net-snmp/library/snmpIPBaseDomain.h >>> create mode 100644 snmplib/transports/snmpIPBaseDomain.c >>> create mode 100644 >>> testing/fulltests/unit-tests/T022netsnmp_parse_ep_str_clib.c >>> >>> -- >>> 2.19.1 >>> >>>
_______________________________________________ Net-snmp-coders mailing list Net-snmp-coders@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
