[ovs-dev] [PATCH] sFlow LAG and Tunnel export

Neil McKee neil.mckee at inmon.com
Mon Mar 10 05:32:40 UTC 2014


The patch below contains all the "spade work" for the new sFlow
extensions,  without making any actual changes to the export.  I am
hoping this can be checked in so that subsequent patches to add the
LAG, Tunnel and OpenFlow-related structures to the export can be small
and targeted.   I hope that's going to be OK?

(And let me know if you would prefer this to be in a new email-thread.)

Regards,
Neil

---

[PATCH] Prepare ground for extensions to sFlow export.
 Standard LACP counters are added to the LACP
 module, and the sFlow library and test modules are extended
 to support the export of those LACP counters as well as
 tunnel and OpenFlow related structures. None of these
 structures are actually exported yet,  so this patch should
 have no discernible effect. Hence no changes to the unit
 tests.

Signed-off-by: Neil McKee <neil.mckee at inmon.com>
---
 lib/lacp.c           |   63 +++++++++++++++++++++++
 lib/lacp.h           |   23 +++++++++
 lib/sflow.h          |   61 ++++++++++++++++++++++-
 lib/sflow_agent.c    |   15 ++++++
 lib/sflow_api.h      |    1 +
 lib/sflow_receiver.c |   39 +++++++++++++++
 tests/test-sflow.c   |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 336 insertions(+), 1 deletion(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index a3f72ed..07f97b8 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -125,6 +125,10 @@ struct slave {
     struct lacp_info ntt_actor;   /* Used to decide if we Need To Transmit. */
     struct timer tx;              /* Next message transmission timer. */
     struct timer rx;              /* Expected message receive timer. */
+
+    uint32_t count_rx_pdus;       /* dot3adAggPortStatsLACPDUsRx */
+    uint32_t count_rx_pdus_bad;   /* dot3adAggPortStatsIllegalRx */
+    uint32_t count_tx_pdus;       /* dot3adAggPortStatsLACPDUsTx */
 };

 static struct ovs_mutex mutex;
@@ -316,9 +320,11 @@ lacp_process_packet(struct lacp *lacp, const void *slave_,
     if (!slave) {
         goto out;
     }
+    slave->count_rx_pdus++;

     pdu = parse_lacp_packet(packet);
     if (!pdu) {
+ slave->count_rx_pdus_bad++;
         VLOG_WARN_RL(&rl, "%s: received an unparsable LACP PDU.", lacp->name);
         goto out;
     }
@@ -536,6 +542,7 @@ lacp_run(struct lacp *lacp, lacp_send_pdu
*send_pdu) OVS_EXCLUDED(mutex)
             slave->ntt_actor = actor;
             compose_lacp_pdu(&actor, &slave->partner, &pdu);
             send_pdu(slave->aux, &pdu, sizeof pdu);
+    slave->count_tx_pdus++;

             duration = (slave->partner.state & LACP_STATE_TIME
                         ? LACP_FAST_TIME_TX
@@ -966,3 +973,59 @@ lacp_unixctl_show(struct unixctl_conn *conn, int
argc, const char *argv[],
 out:
     ovs_mutex_unlock(&mutex);
 }
+
+/* Extract a snapshot of the current state and counters for a slave port.
+   Return false if the slave is not active. */
+bool
+lacp_get_slave_stats(const struct lacp *lacp, const void *slave_,
struct lacp_slave_stats *stats)
+    OVS_EXCLUDED(mutex)
+{
+    struct slave *slave;
+    struct lacp_info actor;
+    bool ret;
+
+    ovs_mutex_lock(&mutex);
+
+    slave = slave_lookup(lacp, slave_);
+    if(slave) {
+ ret = true;
+ slave_get_actor(slave, &actor);
+ memcpy(&stats->dot3adAggPortActorSystemID,
+       actor.sys_id,
+       ETH_ADDR_LEN);
+ memcpy(&stats->dot3adAggPortPartnerOperSystemID,
+       slave->partner.sys_id,
+       ETH_ADDR_LEN);
+ stats->dot3adAggPortAttachedAggID = (lacp->key_slave->key ?
+     lacp->key_slave->key :
+     lacp->key_slave->port_id);
+
+ /* Construct my admin-state.  Assume aggregation is configured on. */
+ stats->dot3adAggPortActorAdminState = LACP_STATE_AGG;
+ if(lacp->active) {
+    stats->dot3adAggPortActorAdminState |= LACP_STATE_ACT;
+ }
+ if(lacp->fast) {
+    stats->dot3adAggPortActorAdminState |= LACP_STATE_TIME;
+ }
+ /* XXX Not sure how to know the partner admin state. It
+ * might have to be captured and remembered during the
+ * negotiation phase.
+ */
+ stats->dot3adAggPortPartnerAdminState = 0;
+
+ stats->dot3adAggPortActorOperState = actor.state;
+ stats->dot3adAggPortPartnerOperState = slave->partner.state;
+
+ /* Read out the latest counters */
+ stats->dot3adAggPortStatsLACPDUsRx = slave->count_rx_pdus;
+ stats->dot3adAggPortStatsIllegalRx = slave->count_rx_pdus_bad;
+ stats->dot3adAggPortStatsLACPDUsTx = slave->count_tx_pdus;
+    }
+    else {
+ ret = false;
+    }
+    ovs_mutex_unlock(&mutex);
+    return ret;
+
+}
diff --git a/lib/lacp.h b/lib/lacp.h
index 593b80d..4295f7b 100644
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -70,4 +70,27 @@ typedef void lacp_send_pdu(void *slave, const void
*pdu, size_t pdu_size);
 void lacp_run(struct lacp *, lacp_send_pdu *);
 void lacp_wait(struct lacp *);

+struct lacp_slave_stats {
+    /* id */
+    uint8_t dot3adAggPortActorSystemID[ETH_ADDR_LEN];
+    uint8_t dot3adAggPortPartnerOperSystemID[ETH_ADDR_LEN];
+    uint32_t dot3adAggPortAttachedAggID;
+    /* state */
+    uint8_t dot3adAggPortActorAdminState;
+    uint8_t dot3adAggPortActorOperState;
+    uint8_t dot3adAggPortPartnerAdminState;
+    uint8_t dot3adAggPortPartnerOperState;
+    /* counters */
+    uint32_t dot3adAggPortStatsLACPDUsRx;
+    /* uint32_t dot3adAggPortStatsMarkerPDUsRx; */
+    /* uint32_t dot3adAggPortStatsMarkerResponsePDUsRx; */
+    /* uint32_t dot3adAggPortStatsUnknownRx; */
+    uint32_t dot3adAggPortStatsIllegalRx;
+    uint32_t dot3adAggPortStatsLACPDUsTx;
+    /* uint32_t dot3adAggPortStatsMarkerPDUsTx; */
+    /* uint32_t dot3adAggPortStatsMarkerResponsePDUsTx; */
+};
+
+bool lacp_get_slave_stats(const struct lacp *, const void *slave_,
struct lacp_slave_stats *);
+
 #endif /* lacp.h */
diff --git a/lib/sflow.h b/lib/sflow.h
index 0d1f2b9..e8ad5c3 100644
--- a/lib/sflow.h
+++ b/lib/sflow.h
@@ -267,6 +267,10 @@ typedef struct _SFLExtended_vlan_tunnel {
     innermost. */
 } SFLExtended_vlan_tunnel;

+typedef struct _SFLExtended_vni {
+    uint32_t vni;            /* virtual network identifier */
+} SFLExtended_vni;
+
 enum SFLFlow_type_tag {
     /* enterprise = 0, format = ... */
     SFLFLOW_HEADER    = 1,      /* Packet headers are sampled */
@@ -285,6 +289,10 @@ enum SFLFlow_type_tag {
     SFLFLOW_EX_MPLS_FTN     = 1010,
     SFLFLOW_EX_MPLS_LDP_FEC = 1011,
     SFLFLOW_EX_VLAN_TUNNEL  = 1012,   /* VLAN stack */
+    SFLFLOW_EX_IPV4_TUNNEL_EGRESS  = 1023, /*
http://sflow.org/sflow_tunnels.txt */
+    SFLFLOW_EX_IPV4_TUNNEL_INGRESS = 1024,
+    SFLFLOW_EX_VNI_EGRESS          = 1029,
+    SFLFLOW_EX_VNI_INGRESS         = 1030,
 };

 typedef union _SFLFlow_type {
@@ -304,6 +312,7 @@ typedef union _SFLFlow_type {
     SFLExtended_mpls_FTN mpls_ftn;
     SFLExtended_mpls_LDP_FEC mpls_ldp_fec;
     SFLExtended_vlan_tunnel vlan_tunnel;
+    SFLExtended_vni tunnel_vni;
 } SFLFlow_type;

 typedef struct _SFLFlow_sample_element {
@@ -382,6 +391,9 @@ typedef struct _SFLFlow_sample_expanded {

 /* Counter types */

+#define SFL_UNDEF_COUNTER(c) c=(typeof(c))-1
+#define SFL_UNDEF_GAUGE(c) c=0
+
 /* Generic interface counters - see RFC 1573, 2233 */

 typedef struct _SFLIf_counters {
@@ -478,6 +490,47 @@ typedef struct _SFLVlan_counters {
     u_int32_t discards;
 } SFLVlan_counters;

+/* OpenFlow port */
+typedef struct {
+    u_int64_t datapath_id;
+    u_int32_t port_no;
+} SFLOpenFlowPort;
+
+/* port name */
+typedef struct {
+    SFLString portName;
+} SFLPortName;
+
+#define SFL_MAX_PORTNAME_LEN 255
+
+/* LAG Port Statistics - see http://sflow.org/sflow_lag.txt */
+/* opaque = counter_data; enterprise = 0; format = 7 */
+
+typedef  union _SFLLACP_portState {
+    uint32_t all;
+    struct {
+ uint8_t actorAdmin;
+ uint8_t actorOper;
+ uint8_t partnerAdmin;
+ uint8_t partnerOper;
+    } v;
+} SFLLACP_portState;
+
+typedef struct _SFLLACP_counters {
+    uint8_t actorSystemID[8];   /* 6 bytes + 2 pad */
+    uint8_t partnerSystemID[8]; /* 6 bytes + 2 pad */
+    uint32_t attachedAggID;
+    SFLLACP_portState portState;
+    uint32_t LACPDUsRx;
+    uint32_t markerPDUsRx;
+    uint32_t markerResponsePDUsRx;
+    uint32_t unknownRx;
+    uint32_t illegalRx;
+    uint32_t LACPDUsTx;
+    uint32_t markerPDUsTx;
+    uint32_t markerResponsePDUsTx;
+} SFLLACP_counters;
+
 /* Counters data */

 enum SFLCounters_type_tag {
@@ -486,7 +539,10 @@ enum SFLCounters_type_tag {
     SFLCOUNTERS_ETHERNET     = 2,
     SFLCOUNTERS_TOKENRING    = 3,
     SFLCOUNTERS_VG           = 4,
-    SFLCOUNTERS_VLAN         = 5
+    SFLCOUNTERS_VLAN         = 5,
+    SFLCOUNTERS_LACP         = 7,
+    SFLCOUNTERS_OPENFLOWPORT = 1004,
+    SFLCOUNTERS_PORTNAME     = 1005
 };

 typedef union _SFLCounters_type {
@@ -495,6 +551,9 @@ typedef union _SFLCounters_type {
     SFLTokenring_counters tokenring;
     SFLVg_counters vg;
     SFLVlan_counters vlan;
+    SFLLACP_counters lacp;
+    SFLOpenFlowPort ofPort;
+    SFLPortName portName;
 } SFLCounters_type;

 typedef struct _SFLCounters_sample_element {
diff --git a/lib/sflow_agent.c b/lib/sflow_agent.c
index 817420d..9c2e028 100644
--- a/lib/sflow_agent.c
+++ b/lib/sflow_agent.c
@@ -363,6 +363,21 @@ SFLPoller *sfl_agent_getPoller(SFLAgent *agent,
SFLDataSource_instance *pdsi)
     return NULL;
 }

+/*_________________-----------------------------------__________________
+  _________________  sfl_agent_getPollerByBridgePort  __________________
+  -----------------___________________________________------------------
+*/
+
+SFLPoller *sfl_agent_getPollerByBridgePort(SFLAgent *agent, uint32_t port_no)
+{
+  /* find it and return it */
+    SFLPoller *pl = agent->pollers;
+    for(; pl != NULL; pl = pl->nxt)
+ if(pl->bridgePort == port_no) return pl;
+    /* not found */
+    return NULL;
+}
+
 /*_________________---------------------------__________________
   _________________  sfl_agent_getReceiver    __________________
   -----------------___________________________------------------
diff --git a/lib/sflow_api.h b/lib/sflow_api.h
index 3cc060b..2730a4c 100644
--- a/lib/sflow_api.h
+++ b/lib/sflow_api.h
@@ -281,6 +281,7 @@ void sfl_agent_set_agentSubId(SFLAgent *agent,
u_int32_t subId);
    to get counters if it is not the same as the global ifIndex */
 void sfl_poller_set_bridgePort(SFLPoller *poller, u_int32_t port_no);
 u_int32_t sfl_poller_get_bridgePort(SFLPoller *poller);
+SFLPoller *sfl_agent_getPollerByBridgePort(SFLAgent *agent, u_int32_t port_no);

 /* call this to indicate a discontinuity with a counter like
samplePool so that the
    sflow collector will ignore the next delta */
diff --git a/lib/sflow_receiver.c b/lib/sflow_receiver.c
index e6fc9a7..937c4fd 100644
--- a/lib/sflow_receiver.c
+++ b/lib/sflow_receiver.c
@@ -464,6 +464,14 @@ static int computeFlowSampleSize(SFLReceiver
*receiver, SFL_FLOW_SAMPLE_TYPE *fs
  case SFLFLOW_EX_MPLS_FTN: elemSiz =
mplsFtnEncodingLength(&elem->flowType.mpls_ftn); break;
  case SFLFLOW_EX_MPLS_LDP_FEC: elemSiz =
mplsLdpFecEncodingLength(&elem->flowType.mpls_ldp_fec); break;
  case SFLFLOW_EX_VLAN_TUNNEL: elemSiz =
vlanTunnelEncodingLength(&elem->flowType.vlan_tunnel); break;
+ case SFLFLOW_EX_IPV4_TUNNEL_EGRESS:
+ case SFLFLOW_EX_IPV4_TUNNEL_INGRESS:
+    elemSiz = sizeof(SFLSampled_ipv4);
+    break;
+ case SFLFLOW_EX_VNI_EGRESS:
+ case SFLFLOW_EX_VNI_INGRESS:
+    elemSiz = sizeof(SFLExtended_vni);
+    break;
  default:
     sflError(receiver, "unexpected packet_data_tag");
     return -1;
@@ -560,6 +568,8 @@ int sfl_receiver_writeFlowSample(SFLReceiver
*receiver, SFL_FLOW_SAMPLE_TYPE *fs
  putNet32(receiver, elem->flowType.ethernet.eth_type);
  break;
     case SFLFLOW_IPV4:
+    case SFLFLOW_EX_IPV4_TUNNEL_EGRESS:
+    case SFLFLOW_EX_IPV4_TUNNEL_INGRESS:
  putNet32(receiver, elem->flowType.ipv4.length);
  putNet32(receiver, elem->flowType.ipv4.protocol);
  put32(receiver, elem->flowType.ipv4.src_ip.addr);
@@ -591,6 +601,11 @@ int sfl_receiver_writeFlowSample(SFLReceiver
*receiver, SFL_FLOW_SAMPLE_TYPE *fs
     case SFLFLOW_EX_MPLS_FTN: putMplsFtn(receiver,
&elem->flowType.mpls_ftn); break;
     case SFLFLOW_EX_MPLS_LDP_FEC: putMplsLdpFec(receiver,
&elem->flowType.mpls_ldp_fec); break;
     case SFLFLOW_EX_VLAN_TUNNEL: putVlanTunnel(receiver,
&elem->flowType.vlan_tunnel); break;
+    case SFLFLOW_EX_VNI_EGRESS:
+    case SFLFLOW_EX_VNI_INGRESS:
+ putNet32(receiver, elem->flowType.tunnel_vni.vni);
+ break;
+
     default:
  sflError(receiver, "unexpected packet_data_tag");
  return -1;
@@ -634,6 +649,9 @@ static int computeCountersSampleSize(SFLReceiver
*receiver, SFL_COUNTERS_SAMPLE_
  case SFLCOUNTERS_TOKENRING: elemSiz =
sizeof(elem->counterBlock.tokenring); break;
  case SFLCOUNTERS_VG: elemSiz = sizeof(elem->counterBlock.vg); break;
  case SFLCOUNTERS_VLAN: elemSiz = sizeof(elem->counterBlock.vlan); break;
+ case SFLCOUNTERS_LACP: elemSiz = sizeof(elem->counterBlock.lacp); break;
+ case SFLCOUNTERS_OPENFLOWPORT: elemSiz =
sizeof(&elem->counterBlock.ofPort); break;
+ case SFLCOUNTERS_PORTNAME: elemSiz =
stringEncodingLength(&elem->counterBlock.portName.portName); break;
  default:
     sflError(receiver, "unexpected counters_tag");
     return -1;
@@ -735,6 +753,27 @@ int sfl_receiver_writeCountersSample(SFLReceiver
*receiver, SFL_COUNTERS_SAMPLE_
  putNet32(receiver, elem->counterBlock.vlan.broadcastPkts);
  putNet32(receiver, elem->counterBlock.vlan.discards);
  break;
+    case SFLCOUNTERS_LACP:
+ putMACAddress(receiver, elem->counterBlock.lacp.actorSystemID);
+ putMACAddress(receiver, elem->counterBlock.lacp.partnerSystemID);
+ putNet32(receiver, elem->counterBlock.lacp.attachedAggID);
+ put32(receiver, elem->counterBlock.lacp.portState.all);
+ putNet32(receiver, elem->counterBlock.lacp.LACPDUsRx);
+ putNet32(receiver, elem->counterBlock.lacp.markerPDUsRx);
+ putNet32(receiver, elem->counterBlock.lacp.markerResponsePDUsRx);
+ putNet32(receiver, elem->counterBlock.lacp.unknownRx);
+ putNet32(receiver, elem->counterBlock.lacp.illegalRx);
+ putNet32(receiver, elem->counterBlock.lacp.LACPDUsTx);
+ putNet32(receiver, elem->counterBlock.lacp.markerPDUsTx);
+ putNet32(receiver, elem->counterBlock.lacp.markerResponsePDUsTx);
+ break;
+    case SFLCOUNTERS_OPENFLOWPORT:
+ putNet64(receiver, &elem->counterBlock.ofPort.datapath_id);
+ putNet32(receiver, &elem->counterBlock.ofPort.port_no);
+ break;
+    case SFLCOUNTERS_PORTNAME:
+ putString(receiver, &elem->counterBlock.portName.portName);
+ break;
     default:
  sflError(receiver, "unexpected counters_tag");
  return -1;
diff --git a/tests/test-sflow.c b/tests/test-sflow.c
index deebd82..6eb4f55 100644
--- a/tests/test-sflow.c
+++ b/tests/test-sflow.c
@@ -53,8 +53,18 @@ static unixctl_cb_func test_sflow_exit;

 /* Structure element tag numbers. */
 #define SFLOW_TAG_CTR_IFCOUNTERS 1
+#define SFLOW_TAG_CTR_LACPCOUNTERS 7
+#define SFLOW_TAG_CTR_OPENFLOWPORT 1004
+#define SFLOW_TAG_CTR_PORTNAME 1005
 #define SFLOW_TAG_PKT_HEADER 1
 #define SFLOW_TAG_PKT_SWITCH 1001
+#define SFLOW_TAG_PKT_TUNNEL4_OUT 1023
+#define SFLOW_TAG_PKT_TUNNEL4_IN 1024
+#define SFLOW_TAG_PKT_TUNNEL_VNI_OUT 1029
+#define SFLOW_TAG_PKT_TUNNEL_VNI_IN 1030
+
+/* string sizes */
+#define SFL_MAX_PORTNAME_LEN 255

 struct sflow_addr {
     enum {
@@ -98,7 +108,14 @@ struct sflow_xdr {
     struct {
         uint32_t HEADER;
         uint32_t SWITCH;
+ uint32_t TUNNEL4_OUT;
+ uint32_t TUNNEL4_IN;
+ uint32_t TUNNEL_VNI_OUT;
+ uint32_t TUNNEL_VNI_IN;
         uint32_t IFCOUNTERS;
+ uint32_t LACPCOUNTERS;
+ uint32_t OPENFLOWPORT;
+ uint32_t PORTNAME;
     } offset;

     /* Flow sample fields. */
@@ -220,6 +237,63 @@ process_counter_sample(struct sflow_xdr *x)
         printf(" promiscuous=%"PRIu32, sflowxdr_next(x));
         printf("\n");
     }
+    if(x->offset.LACPCOUNTERS) {
+ uint8_t *mac;
+ union {
+    uint32_t all;
+    struct {
+ uint8_t actorAdmin;
+ uint8_t actorOper;
+ uint8_t partnerAdmin;
+ uint8_t partnerOper;
+    } v;
+ } state;
+
+        sflowxdr_setc(x, x->offset.LACPCOUNTERS);
+        printf("LACPCOUNTERS");
+ mac = (uint8_t *)sflowxdr_str(x);
+ printf(" sysID="ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+ sflowxdr_skip(x, 2);
+ mac = (uint8_t *)sflowxdr_str(x);
+ printf(" partnerID="ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+ sflowxdr_skip(x, 2);
+ printf(" aggID=%"PRIu32, sflowxdr_next(x));
+ state.all = sflowxdr_next_n(x);
+ printf(" actorAdmin=0x%"PRIx32, state.v.actorAdmin);
+ printf(" actorOper=0x%"PRIx32, state.v.actorOper);
+ printf(" partnerAdmin=0x%"PRIx32, state.v.partnerAdmin);
+ printf(" partnerOper=0x%"PRIx32, state.v.partnerOper);
+ printf(" LACPUDsRx=%"PRIu32, sflowxdr_next(x));
+ printf(" markerPDUsRx=%"PRIu32, sflowxdr_next(x));
+ printf(" markerRespPDUsRx=%"PRIu32, sflowxdr_next(x));
+ printf(" unknownRx=%"PRIu32, sflowxdr_next(x));
+ printf(" illegalRx=%"PRIu32, sflowxdr_next(x));
+ printf(" LACPUDsTx=%"PRIu32, sflowxdr_next(x));
+ printf(" markerPDUsTx=%"PRIu32, sflowxdr_next(x));
+ printf(" markerRespPDUsTx=%"PRIu32, sflowxdr_next(x));
+        printf("\n");
+    }
+    if(x->offset.OPENFLOWPORT) {
+        sflowxdr_setc(x, x->offset.OPENFLOWPORT);
+        printf("OPENFLOWPORT");
+        printf(" datapath_id=%"PRIu64, sflowxdr_next_int64(x));
+        printf(" port_no=%"PRIu32, sflowxdr_next(x));
+ printf("\n");
+    }
+    if(x->offset.PORTNAME) {
+ uint32_t pnLen;
+ char *pnBytes;
+ char portName[SFL_MAX_PORTNAME_LEN + 1];
+        sflowxdr_setc(x, x->offset.PORTNAME);
+        printf("PORTNAME");
+ pnLen = sflowxdr_next(x);
+ SFLOWXDR_assert(x, (pnLen <= SFL_MAX_PORTNAME_LEN));
+ pnBytes = sflowxdr_str(x);
+ memcpy(portName, pnBytes, pnLen);
+ portName[pnLen] = '\0';
+ printf(" portName=%s", portName);
+ printf("\n");
+    }
 }

 static char
@@ -250,6 +324,25 @@ print_hex(const char *a, int len, char *buf, int bufLen)
     return b;
 }

+static void
+print_struct_ipv4(struct sflow_xdr *x, const char *prefix)
+{
+    uint32_t src, dst;
+
+    printf(" %s_length=%"PRIu32,    prefix, sflowxdr_next(x));
+    printf(" %s_protocol=%"PRIu32,  prefix, sflowxdr_next(x));
+
+    src = sflowxdr_next_n(x);
+    dst = sflowxdr_next_n(x);
+    printf(" %s_src="IP_FMT,        prefix, IP_ARGS(src));
+    printf(" %s_dst="IP_FMT,        prefix, IP_ARGS(dst));
+
+    printf(" %s_src_port=%"PRIu32,  prefix, sflowxdr_next(x));
+    printf(" %s_dst_port=%"PRIu32,  prefix, sflowxdr_next(x));
+    printf(" %s_tcp_flags=%"PRIu32, prefix, sflowxdr_next(x));
+    printf(" %s_tos=%"PRIu32,       prefix, sflowxdr_next(x));
+}
+
 #define SFLOW_HEX_SCRATCH 1024

 static void
@@ -265,6 +358,26 @@ process_flow_sample(struct sflow_xdr *x)
                x->agentIPStr, x->dsClass, x->dsIndex);
         printf(" fsSeqNo=%"PRIu32, x->fsSeqNo);

+        if (x->offset.TUNNEL4_IN) {
+            sflowxdr_setc(x, x->offset.TUNNEL4_IN);
+    print_struct_ipv4(x, "tunnel4_in");
+        }
+
+        if (x->offset.TUNNEL4_OUT) {
+            sflowxdr_setc(x, x->offset.TUNNEL4_OUT);
+    print_struct_ipv4(x, "tunnel4_out");
+        }
+
+        if (x->offset.TUNNEL_VNI_IN) {
+            sflowxdr_setc(x, x->offset.TUNNEL_VNI_IN);
+    printf( " tunnel_in_vni=%"PRIu32, sflowxdr_next(x));
+        }
+
+        if (x->offset.TUNNEL_VNI_OUT) {
+            sflowxdr_setc(x, x->offset.TUNNEL_VNI_OUT);
+    printf( " tunnel_out_vni=%"PRIu32, sflowxdr_next(x));
+        }
+
         if (x->offset.SWITCH) {
             sflowxdr_setc(x, x->offset.SWITCH);
             printf(" in_vlan=%"PRIu32, sflowxdr_next(x));
@@ -371,6 +484,12 @@ process_datagram(struct sflow_xdr *x)
                 case SFLOW_TAG_CTR_IFCOUNTERS:
                     sflowxdr_mark_unique(x, &x->offset.IFCOUNTERS);
                     break;
+                case SFLOW_TAG_CTR_LACPCOUNTERS:
+                    sflowxdr_mark_unique(x, &x->offset.LACPCOUNTERS);
+                    break;
+                case SFLOW_TAG_CTR_PORTNAME:
+                    sflowxdr_mark_unique(x, &x->offset.PORTNAME);
+                    break;

                     /* Add others here... */
                 }
@@ -439,6 +558,22 @@ process_datagram(struct sflow_xdr *x)
                     sflowxdr_mark_unique(x, &x->offset.SWITCH);
                     break;

+ case SFLOW_TAG_PKT_TUNNEL4_OUT:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL4_OUT);
+                    break;
+
+ case SFLOW_TAG_PKT_TUNNEL4_IN:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL4_IN);
+                    break;
+
+ case SFLOW_TAG_PKT_TUNNEL_VNI_OUT:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL_VNI_OUT);
+                    break;
+
+ case SFLOW_TAG_PKT_TUNNEL_VNI_IN:
+                    sflowxdr_mark_unique(x, &x->offset.TUNNEL_VNI_IN);
+                    break;
+
                     /* Add others here... */
                 }

-- 
1.7.9.5
------
Neil McKee
InMon Corp.
http://www.inmon.com


On Wed, Feb 19, 2014 at 8:39 AM, Neil McKee <neil.mckee at inmon.com> wrote:
> Finally getting back to this.  Can we do it one piece at a time?  For
> example,  attached is a patch that just adds counters to the LACP
> module and a call to retrieve them.
>
> Neil
> ------
> Neil McKee
> InMon Corp.
> http://www.inmon.com
>
>
> On Mon, Jan 13, 2014 at 8:29 AM, Neil McKee <neil.mckee at inmon.com> wrote:
>> Thanks for the detailed explanation.  You should teach a class :)    I'll
>> rework the patch.
>>
>> Neil
>>
>>
>>
>> ------
>> Neil McKee
>> InMon Corp.
>> http://www.inmon.com
>>
>>
>> On Thu, Jan 9, 2014 at 11:43 AM, Ben Pfaff <blp at nicira.com> wrote:
>>>
>>> On Fri, Jan 03, 2014 at 09:39:00PM -0800, Neil McKee wrote:
>>> > On Mon, Dec 30, 2013 at 4:08 PM, Ben Pfaff <blp at nicira.com> wrote:
>>> > > I'm not a fan of callbacks because of the surprising inversion of
>>> > > control that they cause, and especially not of layered callbacks like
>>> > > the one that this introduces.  Instead of calling back into
>>> > > dpif_sflow_port_lookup_callback() from sflow_agent_get_counters(), I
>>> > > think that it would be better to provide the necessary information in
>>> > > the call to dpif_sflow_add_port().
>>> > >
>>> > >
>>> > I wasn't expecting you to say that.  The purpose of the callback is
>>> > really just to access the state held within ofproto-dpif,  but in an
>>> > encapsulated way so that the exposure is limited and controlled.
>>> > The alternatives were (1) to make those private structure fields
>>> > visible to the ofproto-dpif-sflow module so it could just read them
>>> > directly, or (2) to copy all the relevant state into a separate
>>> > hash-table just for ofproto-dpif-sflow to use when processing packet
>>> > samples. It sounds like you prefer (2)?   To understand why I think
>>> > I just need to know what you mean by "surprising inversion of control".
>>> > Are you referring to the possibility of mutex contention/deadlock
>>> > if the data-structures within ofproto-dpif are interrogated from
>>> > within the packet-processing path (i.e. when processing an
>>> > sFlow packet sample)?
>>>
>>> Let me step back and explain, since my phrasing didn't help.  I'm
>>> going to explain one of my own views of program structure.  Most of it
>>> is opinion, not fact, so bear with me a little.
>>>
>>> The call stack in a well-structured large program usually follows a
>>> tree structure from one logical module (often, one source file) to
>>> another.  Often, you can consider one module to be "higher level" or
>>> "lower level" than another based on the rule that higher level modules
>>> call into lower level modules, but not vice versa.  Sometimes calls go
>>> both ways, and in my experience that is undesirable because it makes
>>> the structure of the program harder to understand.  Direct calls are
>>> one thing, because it is easy enough to trace through them with "grep"
>>> and other tools, but indirect calls through callbacks are harder to
>>> understand.  Actually, there are at least two classes of callbacks:
>>> callbacks that you pass into a function for that function to call, and
>>> callbacks that you store somewhere in data owned by a module that in
>>> theory any function in that module *could* call (even if it doesn't
>>> currently).  The latter are harder to understand.
>>>
>>> My personal idea of nightmarish tangles of callbacks is GObject and in
>>> particular GTK+.  Everything is a callback and it's very difficult to
>>> figure out exactly when one of them could get called.  You end up
>>> writing bad code because you can't figure it out.  (GTK+ has other
>>> problems too, that's just one of them.)
>>>
>>> Looking at the callback in question again, it's going to be difficult
>>> to maintain for a reason that you basically called out in the comment,
>>> that is, locking.  In most places in OVS we are able to use Clang
>>> thread safety annotations to statically check (to some basic level of
>>> safety, anyhow) that locks are held or not held at various places.
>>> But those checks can't flow through callback functions:
>>>
>>>     /* XXX do we need to acquire ofproto_mutex here?  Or should it be
>>> acquired in handle_sflow_upcall() below,
>>>      * so that it will not be released until after the sFlow-module has
>>> followed the pointers we are giving
>>>      * it here and the sflow sample has been fully processed? */
>>>
>>> So I'd prefer, if we can avoid it, to use some interface more direct
>>> than a callback.
>>
>>



More information about the dev mailing list