[Net-snmp-patches] [ net-snmp-Patches-3394544 ] ability to specify local-bound port in addition to address

[SourceForge.net <no...@so...>]

Patches item #3394544, was opened at 2011-08-19 16:07
Message generated for change (Comment added) made by iraisr
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312694&aid=3394544&group_id=12694

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Ivosh (iraisr)
>Assigned to: Wes Hardaker (hardaker)
Summary: ability to specify local-bound port in addition to address

Initial Comment:
Hi,
here is the patch against net-snmp-5.7.1.pre1 which adds the ability into UDP IPv4 transport to specifically choose which local port to bind on.
Previously, there was an option NETSNMP_DS_LIB_CLIENT_ADDR to specify local address (which to bind to).
However, in specific cases where for example traps/notifications must be sent from the application from specific IP address _AND_ port, this was not possible. So now there is another option NETSNMP_DS_LIB_CLIENT_PORT which takes the local port to bind to. If this option is not specified (implicit 0), then the previous behaviour (OS chooses) is in place.

Tested on Linux/x86_64 and Linux/i686, RHEL5. Should be ok on all other systems, though.
Let me know if there are any problems or hurdles. Otherwise I am looking forward to incorporating this patch in upstream as soon as possible.
Kind regards,
Ivo Raisr

----------------------------------------------------------------------

>Comment By: Ivosh (iraisr)
Date: 2011-08-31 08:15

Message:
Hi Wes,
I agree there could be problems in multi-threaded applications.
However, I closely followed the appropach which is already in place for
NETSNMP_DS_LIB_CLIENT_ADDR.

Therefore if I understood you correctly, you are suggesting to:
- do not use NETSNMP_DS_LIB_CLIENT_PORT and NETSNMP_DS_LIB_CLIENT_ADDR
alltogether
- create transport config options structure which gets passed to all
transport_* functions
- fill this config structure from passed-in session in function_sess_open

The problem, however, remains in how to pass the config options to
transport_* functions.
For example netsnmp_tdomain_transport_full takes only the following
arguments:
- application, remote address, local port, domain and target.
Subsequent callback f_create_from_tstring_new takes only the following
arguments:
- address (either local or remote), local port and target
However functionality in netsnmp_udpipv4base_transport and
netsnmp_udp6_transport functions is so twisted that "local port" is also
used to switch between server and client side transport.
Things start to get more complicated...

---------

Another option which may get us to the goal is to allow "address:port"
specifiers for NETSNMP_DS_LIB_CLIENT_ADDR instead of just "address".
Function netsnmp_sockaddr_in2 which gets called from
netsnmp_udpipv4base_transport already counts with this possibility.
The only problem is that if NETSNMP_DS_LIB_CLIENT_ADDR did not contain the
port, netsnmp_sockaddr_in2 would override it with the default (161 I
think).
Therefore I propose to add another global option
NETSNMP_DS_LIB_CLIENT_ADDR_USES_PORT to overcome this problem:
- if it is false (the default), then "client_addr.sin_port = 0;" remains
in place in function netsnmp_udpipv4base_transport and everything works as
before
- if it is true, then netsnmp_sockaddr_in2 will use the port and
"client_addr.sin_port = 0;" will not be in effect; therefore achieving the
desired functionality

One can see that proposed option NETSNMP_DS_LIB_CLIENT_ADDR_USES_PORT is
not dynamic and will be typically set once per whole application.

I would be glad to hear your comments and thoughts.
Kind regards, Ivosh

----------------------------------------------------------------------

Comment By: Wes Hardaker (hardaker)
Date: 2011-08-30 19:20

Message:
I'm not applying this at the moment since it appears unsafe to use in a
threaded situation.  In particular, the code in the UDP transport must make
use of the global default_store for the port, which might be changing by
more than one thread at a time.

I think it'd be better to create a transport config options to pass in the
local port instead and use that.

On the upside, it's wonderful you submitted a patch even to the perl
modules after updating the DS enums.  *no one* does that!

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=312694&aid=3394544&group_id=12694