Re: [PATCH 0/6] Add support for interface binding

[Bill F. <fe...@us...>]

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 <bva...@ac...> 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
>
>