On Wed, Sep 07, 2005 at 04:06:13PM +0100, Dave Shield wrote:
> On Tue, 2005-09-06 at 00:25 +0200, Magnus Fromreide wrote:
>
> > I added patch #1282566 wich I think implements this.
>
> That's certainly a good start, but I don't think it's quite right.
> A couple of things struck me immediately:
>
> a) Your patch appears to be specific to the agent trap handling.
> This is actually an example of a more general problem,
> and we should be looking for a generic solution.
I know.
See it as two patches, one for the trapsinks (the patch to agent/agent_trap.c)
and one that adds a general framework (everything else)
I have patches for snmptrapd and agentx prepared, and could add them to the
patch tonight.
The one for smux is not finished yet as that involves making smux use
transports in the first place...
Are there any more places that should use another port than the default one?
> b) It looks as if you're changing the existing APIs (e.g.
> the various create_tstring parameters, as well as the
> tdomain structure). This may not matter, but should be
> avoided if at all possible.
It should not matter, but I agree that it made me a little nervous.
For the tdomain structure I add parameters but if I remember correctly it
isn't an error to call a C function with to many arguments, the extra ones
are simply ignored, so thirdparty transports would continue working as they
do today.
That I change the create_tstring functions could be problematic but that could
be worked around by renaming them and adding compatibility functions.
netsnmp_transport*
create_tstring_new(const char *string, int local,
netsnmp_match_fun match, void* user)
netsnmp_transport*
create_tstring(const char *string, int local)
{
return create_tstring_new(string, local, NULL, NULL);
}
> What I'd envisage would be a call such as
>
> netsnmp_transport *
> netsnmp_transport_open( char *string, int def_port )
>
> which would take a transport specification string:
> ("tcp:localhost:1234", "udp:10.0.0.1", "myhost", etc) and
> a default port number (which the first example would ignore),
>
> This routine would then return the appropriate transport
> structure.
Yes, but what is a default port number in an IPX context? The reason I used a
void* for the user argument is so that anything imaginable could be transferred
if an u_short is to limited to describe the default argument.
I am also imagining that for example you could send in "unix:" to the agentx
handler and make it use /var/agentx/master, something your proposal can't do.
Finally you can implement netsnmp_transport_open as:
int
netsnmp_transport_open_match(const char* d, const char* s, void* a, void* u)
{
u_short port = u && *(u_short *)u ? *(u_short *)u : SNMP_TRAP_PORT;
if (strcmp(d, "udp") == 0 || strcmp(d, "tcp") == 0)
return netsnmp_sockaddr_in(a, s, port);
else if (strcmp(d, "udp6") == 0 || strcmp(d, "tcp6") == 0)
return netsnmp_sockaddr_in6(a, s, port);
else
return -1;
}
netsnmp_transport *
netsnmp_transport_open( char *string, int def_port )
{
return netsnmp_tdomain_transport_ex(sink, 0, "udp",
&netsnmp_transport_open_match,
&def_port);
}
> The create_trap_session* routines shouldn't need to bother
> about *any* of that detail - it's purely a matter for the
> library transport code.
Yes, but if the session routines wish to bother about it then we shouldn't stop
them, should we?
/MF
-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
