On Thu, May 24, 2018 at 11:12:12PM +0200, Anders Selhammer wrote: > One mib object is initialized in master agent and data is received > from ptp4l. > > Signed-off-by: Anders Selhammer <[email protected]> > --- > makefile | 4 +- > ptpbase_mib.c | 130 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ptpbase_mib.h | 30 ++++++++++++++ > snmpd.c | 39 ++++++++++++++++++
It is hard for me to see why the code in ptpbase_mib.c needs to be in a seperate file from snmpd.c > diff --git a/ptpbase_mib.c b/ptpbase_mib.c > new file mode 100644 > index 0000000..a73987b > --- /dev/null > +++ b/ptpbase_mib.c > @@ -0,0 +1,130 @@ > +/** > + * @file ptpbase_mib.c > + * @note Copyright (C) 2018 Anders Selhammer <[email protected]> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License along > + * with this program; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > +#include <net-snmp/net-snmp-config.h> > +#include <net-snmp/net-snmp-includes.h> > +#include <net-snmp/agent/net-snmp-agent-includes.h> > + > +#include "print.h" > +#include "ptpbase_mib.h" > + > +long longObject = 0; > +size_t long_size = sizeof(long); This won't scale to many objects of different types. > +struct ptp_message* (*pmc_cb)(char *command); Can't see why this needs to be a callback. Just call run_pmc() directly. > +static void get_mgmt_data(struct ptp_message *msg, > + struct management_tlv **mgt, > + struct tlv_extra **extra) > +{ > + int action; > + struct TLV *tlv; > + > + if (msg_type(msg) != MANAGEMENT) { > + return; > + } > + > + action = management_action(msg); > + if (action < GET || action > ACKNOWLEDGE) { > + return; > + } > + > + if (msg->tlv_count != 1) { > + return; > + } > + > + tlv = (struct TLV *) msg->management.suffix; > + if (tlv->type == TLV_MANAGEMENT) { > + ; > + } else if (tlv->type == TLV_MANAGEMENT_ERROR_STATUS) { > + pr_err("MANAGEMENT_ERROR_STATUS"); > + return; > + } else { > + pr_err("unknown-tlv"); > + return; > + } > + > + *extra = TAILQ_FIRST(&msg->tlv_list); > + *mgt = (struct management_tlv *) msg->management.suffix; > + if ((*mgt)->length == 2 && (*mgt)->id != TLV_NULL_MANAGEMENT) { > + pr_err("empty-tlv"); This error is not propagated back to the caller. > + return; > + } > +} > + > +static void get_msg_val(struct ptp_message *msg) > +{ > + struct management_tlv *mgt = NULL; > + struct management_tlv_datum *mtd; > + struct tlv_extra *extra = NULL; > + > + get_mgmt_data(msg, &mgt, &extra); > + if (!mgt) { > + pr_err("error in pmc msg"); > + return; > + } > + > + switch (mgt->id) { > + case TLV_VERSION_NUMBER: > + mtd = (struct management_tlv_datum *) mgt->data; > + pr_err("VERSION_NUMBER %hhu", mtd->val); This is just for prototyping and debugging, right? > + longObject = mtd->val; Better to pass in a pointer to a data structure that holds the result. Then you can avoid the global. > + break; > + } > +} > + > +static int handle_tempMibObject(netsnmp_mib_handler *handler, Avoid camelCase please. > + netsnmp_handler_registration *reginfo, > + netsnmp_agent_request_info *reqinfo, > + netsnmp_request_info *requests) > +{ > + struct ptp_message *msg; /* no need for global: */ long val; > + switch (reqinfo->mode) { > + case MODE_GET: > + msg = pmc_cb("GET VERSION_NUMBER"); > + if (msg) { > + get_msg_val(msg); > + msg_put(msg); > + } > + snmp_set_var_typed_value(requests->requestvb, ASN_UNSIGNED, > + &longObject, long_size); msg = pmc_cb("GET VERSION_NUMBER"); if (!msg) { return SNMP_ERR_MUMBLE; } get_msg_val(msg, &val); msg_put(msg); err = snmp_set_var_typed_value(requests->requestvb, ASN_UNSIGNED, &val, sizeof(val)); if (err) { ... } return SNMP_ERR_NOERROR; > + break; > + default: > + snmp_log(LOG_ERR, "unknown mode (%d) in handle_totalClients\n", > + reqinfo->mode); > + return SNMP_ERR_GENERR; > + } > + return SNMP_ERR_NOERROR; > +} > + > +static void init_tempMibObject(void) > +{ > + const oid tempMibObject_oid[] = { 1, 3, 6, 1, 2, 1, 241, 1, 2, 8, 1, 15 > }; Use proper defines rather than magic numbers please. > + netsnmp_register_scalar(netsnmp_create_handler_registration( > + "tempMibObject", > + handle_tempMibObject, > + tempMibObject_oid, > + OID_LENGTH(tempMibObject_oid), > + HANDLER_CAN_RONLY)); Don't nest function calls like this. Also, this lacks error handling. > +} > + > +void init_ptpbaseMib(struct ptp_message* (*cb)(char *command)) > +{ > + pmc_cb = cb; Really don't see the point of this callback. Do you expect to pass in different values of 'cb' ? > + init_tempMibObject(); > +} Thanks, Richard ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linuxptp-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
