On 09/06/2017 02:53 AM, Weglicki, MichalX wrote:
Hey Greg,

Do you have any schedule for checking this patch?

Thank you in advance!

Br,
Michal.

Hi Michal,

I'll work on it this week and see if I can resolve the connection problem to 
the collector.

Thanks,

- Greg


-----Original Message-----
From: Greg Rose [mailto:gvrose8...@gmail.com]
Sent: Tuesday, August 29, 2017 5:15 PM
To: Weglicki, MichalX <michalx.wegli...@intel.com>
Cc: d...@openvswitch.org; Szczerbik, PrzemyslawX 
<przemyslawx.szczer...@intel.com>
Subject: Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface 
Information Elements to flow key

On 08/29/2017 04:48 AM, Weglicki, MichalX wrote:
Hello Greg,

Unfortunately Przemek is not available, but I'm more or less familiar with this 
patch.

Did you manage to fix this issue? I think that your problems lies on connection 
level, which wasn't modified by this patch. Patch
added additional counters on reporting level, and based on your description, 
ovs just can't connect to collector.

Br,
Michal.

Correct - it is not an issue with the patch.  However, I need to test the patch 
and ran into problems while trying to do so.  Since then
I've been pulled into some other work and will get back to this afterwards.  
I'll just need to debug the connection issue to the
collector.

Thanks,

- Greg


-----Original Message-----
From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] 
On Behalf Of Greg Rose
Sent: Saturday, August 19, 2017 12:51 AM
To: Szczerbik, PrzemyslawX <przemyslawx.szczer...@intel.com>
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface 
Information Elements to flow key

On 08/16/2017 01:54 AM, Szczerbik, PrzemyslawX wrote:
Hi,

I haven't received any feedback on this patch for quite some time.
Is there anything that I can do to expedite review process?

Regards,
Przemek


Przemek,

I'm in the process of looking into this patch but I'm running into an issue 
with the vswitch not sending flows to the
ntopng/nprobe
collector I have set up.  I see this in the vswitchd log:

2017-08-18T21:48:57.917Z|00058|collectors|WARN|couldn't open connection to 
collector 192.168.0.21:2055 (Network is
unreachable)
2017-08-18T21:48:57.917Z|00059|ipfix|WARN|no collectors could be initialized, 
IPFIX exporter disabled

However, 192.168.0.21 is reachable, at least from the br0 bridge that has the 
IPFIX flows enabled.

ntopng/nprobe on the collector machine has the right ports open:

netstat -tulpen | grep 2055
udp        0      0 0.0.0.0:2055            0.0.0.0:*                           
99         27671      3038/nprobe
udp6       0      0 :::2055                 :::*                                
99         27672      3038/nprobe

netstat -tulpen | grep 5556
tcp        0      0 0.0.0.0:5556            0.0.0.0:*               LISTEN      
0          27666      3038/nprobe

I'm not sure exactly what the problem is but I'm debugging that.  Until I can 
get past this issue I can't really test and review your
patch.  I am actively working on getting this issue fixed though.

Regards,

- Greg
-----Original Message-----
From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
boun...@openvswitch.org] On Behalf Of Szczerbik, PrzemyslawX
Sent: Wednesday, July 26, 2017 12:01 PM
To: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information
Elements to flow key

Hi,

This patch was supposed to be v2, but I forgot to mention that in the subject.
Previous version: https://patchwork.ozlabs.org/patch/792730/

Let me know if you want me to re-sent it with a proper version.

Regards,
Przemek

-----Original Message-----
From: Szczerbik, PrzemyslawX
Sent: Wednesday, July 26, 2017 10:44 AM
To: d...@openvswitch.org
Cc: Szczerbik, PrzemyslawX <przemyslawx.szczer...@intel.com>
Subject: [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to
flow key

Extend flow key part of data record to include following Information Elements:
- ingressInterface
- ingressInterfaceType
- egressInterface
- egressInterfaceType
- interfaceName
- interfaceDescription

In case of input sampling we don't have information about egress port.
Define templates depending not only on protocol types, but also on flow
direction. Only egress flow will include egress information elements.

With this change, dpif_ipfix_exporter stores every port in hmap rather
than only tunnel ports. It allows to easily retrieve required
information about interfaces during sampling upcalls.

Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczer...@intel.com>
---
v1->v2
* Add interfaceType and interfaceDescription
* Rework ipfix_get_iface_data_record function

    ofproto/ofproto-dpif-ipfix.c | 356 +++++++++++++++++++++++++++++++-----
--
-----
    ofproto/ofproto-dpif-ipfix.h |   6 +-
    ofproto/ofproto-dpif-xlate.c |   4 +-
    ofproto/ofproto-dpif.c       |  19 +--
    4 files changed, 275 insertions(+), 110 deletions(-)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 13ff426..e7ce279 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -113,11 +113,12 @@ struct dpif_ipfix_global_stats {
    };

    struct dpif_ipfix_port {
-    struct hmap_node hmap_node; /* In struct dpif_ipfix's "tunnel_ports"
hmap.
*/
+    struct hmap_node hmap_node; /* In struct dpif_ipfix's "ports" hmap. */
        struct ofport *ofport;      /* To retrieve port stats. */
        odp_port_t odp_port;
        enum dpif_ipfix_tunnel_type tunnel_type;
        uint8_t tunnel_key_length;
+    uint32_t ifindex;
    };

    struct dpif_ipfix_exporter {
@@ -155,9 +156,9 @@ struct dpif_ipfix_flow_exporter_map_node {
    struct dpif_ipfix {
        struct dpif_ipfix_bridge_exporter bridge_exporter;
        struct hmap flow_exporter_map;  /* dpif_ipfix_flow_exporter_map_node.
*/
-    struct hmap tunnel_ports;       /* Contains "struct dpif_ipfix_port"s.
-                                     * It makes tunnel port lookups faster in
-                                     * sampling upcalls. */
+    struct hmap ports;              /* Contains "struct dpif_ipfix_port"s.
+                                     * It makes port lookups faster in sampling
+                                     * upcalls. */
        struct ovs_refcount ref_cnt;
    };

@@ -291,7 +292,8 @@ BUILD_ASSERT_DECL(sizeof(struct
ipfix_template_field_specifier) == 8);
    /* Cf. IETF RFC 5102 Section 5.11.6. */
    enum ipfix_flow_direction {
        INGRESS_FLOW = 0x00,
-    EGRESS_FLOW = 0x01
+    EGRESS_FLOW = 0x01,
+    NUM_IPFIX_FLOW_DIRECTION
    };

    /* Part of data record flow key for common metadata and Ethernet entities. 
*/
@@ -306,6 +308,18 @@ struct ipfix_data_record_flow_key_common {
    });
    BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_common) ==
20);

+/* Part of data record flow key for interface information. Since some of the
+ * elements have variable length, members of this structure should be
appended
+ * to the 'struct dp_packet' one by one. */
+struct ipfix_data_record_flow_key_iface {
+    ovs_be32 if_index;     /* (INGRESS|EGRESS)_INTERFACE */
+    ovs_be32 if_type;     /* (INGRESS|EGRESS)_INTERFACE_TYPE */
+    uint8_t if_name_len;   /* Variable length element: INTERFACE_NAME */
+    char *if_name;
+    uint8_t if_descr_len; /* Variable length element: INTERFACE_DESCRIPTION
*/
+    char *if_descr;
+};
+
    /* Part of data record flow key for VLAN entities. */
    OVS_PACKED(
    struct ipfix_data_record_flow_key_vlan {
@@ -735,7 +749,7 @@ dpif_ipfix_find_port(const struct dpif_ipfix *di,
        struct dpif_ipfix_port *dip;

        HMAP_FOR_EACH_IN_BUCKET (dip, hmap_node,
hash_odp_port(odp_port),
-                             &di->tunnel_ports) {
+                             &di->ports) {
            if (dip->odp_port == odp_port) {
                return dip;
            }
@@ -744,82 +758,114 @@ dpif_ipfix_find_port(const struct dpif_ipfix *di,
    }

    static void
-dpif_ipfix_del_port(struct dpif_ipfix *di,
+dpif_ipfix_del_port__(struct dpif_ipfix *di,
                          struct dpif_ipfix_port *dip)
        OVS_REQUIRES(mutex)
    {
-    hmap_remove(&di->tunnel_ports, &dip->hmap_node);
+    hmap_remove(&di->ports, &dip->hmap_node);
        free(dip);
    }

+static enum dpif_ipfix_tunnel_type
+dpif_ipfix_tunnel_type(const struct ofport *ofport) {
+    const char *type = netdev_get_type(ofport->netdev);
+
+    if (type == NULL) {
+        return DPIF_IPFIX_TUNNEL_UNKNOWN;
+    }
+    if (strcmp(type, "gre") == 0) {
+        return DPIF_IPFIX_TUNNEL_GRE;
+    } else if (strcmp(type, "vxlan") == 0) {
+        return DPIF_IPFIX_TUNNEL_VXLAN;
+    } else if (strcmp(type, "lisp") == 0) {
+        return DPIF_IPFIX_TUNNEL_LISP;
+    } else if (strcmp(type, "geneve") == 0) {
+        return DPIF_IPFIX_TUNNEL_GENEVE;
+    } else if (strcmp(type, "stt") == 0) {
+        return DPIF_IPFIX_TUNNEL_STT;
+    }
+
+    return DPIF_IPFIX_TUNNEL_UNKNOWN;
+}
+
+static uint8_t
+dpif_ipfix_tunnel_key_length(enum dpif_ipfix_tunnel_type tunnel_type) {
+
+    switch (tunnel_type) {
+        case DPIF_IPFIX_TUNNEL_GRE:
+            /* 32-bit key gre */
+            return 4;
+        case DPIF_IPFIX_TUNNEL_VXLAN:
+        case DPIF_IPFIX_TUNNEL_LISP:
+        case DPIF_IPFIX_TUNNEL_GENEVE:
+            return 3;
+        case DPIF_IPFIX_TUNNEL_STT:
+            return 8;
+        case DPIF_IPFIX_TUNNEL_UNKNOWN:
+        case NUM_DPIF_IPFIX_TUNNEL:
+        default:
+            return 0;
+    }
+}
+
    void
-dpif_ipfix_add_tunnel_port(struct dpif_ipfix *di, struct ofport *ofport,
-                           odp_port_t odp_port) OVS_EXCLUDED(mutex)
+dpif_ipfix_add_port(struct dpif_ipfix *di, struct ofport *ofport,
+                    odp_port_t odp_port) OVS_EXCLUDED(mutex)
    {
        struct dpif_ipfix_port *dip;
-    const char *type;
+    int64_t ifindex;

        ovs_mutex_lock(&mutex);
        dip = dpif_ipfix_find_port(di, odp_port);
        if (dip) {
-        dpif_ipfix_del_port(di, dip);
+        dpif_ipfix_del_port__(di, dip);
        }

-    type = netdev_get_type(ofport->netdev);
-    if (type == NULL) {
-        goto out;
+    ifindex = netdev_get_ifindex(ofport->netdev);
+    if (ifindex < 0) {
+        ifindex = 0;
        }

-    /* Add to table of tunnel ports. */
+    /* Add to table of ports. */
        dip = xmalloc(sizeof *dip);
        dip->ofport = ofport;
        dip->odp_port = odp_port;
-    if (strcmp(type, "gre") == 0) {
-        /* 32-bit key gre */
-        dip->tunnel_type = DPIF_IPFIX_TUNNEL_GRE;
-        dip->tunnel_key_length = 4;
-    } else if (strcmp(type, "vxlan") == 0) {
-        dip->tunnel_type = DPIF_IPFIX_TUNNEL_VXLAN;
-        dip->tunnel_key_length = 3;
-    } else if (strcmp(type, "lisp") == 0) {
-        dip->tunnel_type = DPIF_IPFIX_TUNNEL_LISP;
-        dip->tunnel_key_length = 3;
-    } else if (strcmp(type, "geneve") == 0) {
-        dip->tunnel_type = DPIF_IPFIX_TUNNEL_GENEVE;
-        dip->tunnel_key_length = 3;
-    } else if (strcmp(type, "stt") == 0) {
-        dip->tunnel_type = DPIF_IPFIX_TUNNEL_STT;
-        dip->tunnel_key_length = 8;
-    } else {
-        free(dip);
-        goto out;
-    }
-    hmap_insert(&di->tunnel_ports, &dip->hmap_node,
hash_odp_port(odp_port));
+    dip->tunnel_type = dpif_ipfix_tunnel_type(ofport);
+    dip->tunnel_key_length = dpif_ipfix_tunnel_key_length(dip-
tunnel_type);
+    dip->ifindex = ifindex;
+    hmap_insert(&di->ports, &dip->hmap_node, hash_odp_port(odp_port));

-out:
        ovs_mutex_unlock(&mutex);
    }

    void
-dpif_ipfix_del_tunnel_port(struct dpif_ipfix *di, odp_port_t odp_port)
+dpif_ipfix_del_port(struct dpif_ipfix *di, odp_port_t odp_port)
        OVS_EXCLUDED(mutex)
    {
        struct dpif_ipfix_port *dip;
        ovs_mutex_lock(&mutex);
        dip = dpif_ipfix_find_port(di, odp_port);
        if (dip) {
-        dpif_ipfix_del_port(di, dip);
+        dpif_ipfix_del_port__(di, dip);
        }
        ovs_mutex_unlock(&mutex);
    }

+static struct dpif_ipfix_port *
+dpif_ipfix_find_tunnel_port(const struct dpif_ipfix *di, odp_port_t odp_port)
+    OVS_REQUIRES(mutex)
+{
+    struct dpif_ipfix_port *dip = dpif_ipfix_find_port(di, odp_port);
+    return (dip && dip->tunnel_type != DPIF_IPFIX_TUNNEL_UNKNOWN) ? dip
:
NULL;
+}
+
    bool
-dpif_ipfix_get_tunnel_port(const struct dpif_ipfix *di, odp_port_t odp_port)
+dpif_ipfix_is_tunnel_port(const struct dpif_ipfix *di, odp_port_t odp_port)
        OVS_EXCLUDED(mutex)
    {
        struct dpif_ipfix_port *dip;
        ovs_mutex_lock(&mutex);
-    dip = dpif_ipfix_find_port(di, odp_port);
+    dip = dpif_ipfix_find_tunnel_port(di, odp_port);
        ovs_mutex_unlock(&mutex);
        return dip != NULL;
    }
@@ -1055,7 +1101,7 @@ dpif_ipfix_create(void)
        di = xzalloc(sizeof *di);
        dpif_ipfix_bridge_exporter_init(&di->bridge_exporter);
        hmap_init(&di->flow_exporter_map);
-    hmap_init(&di->tunnel_ports);
+    hmap_init(&di->ports);
        ovs_refcount_init(&di->ref_cnt);
        return di;
    }
@@ -1149,8 +1195,8 @@ dpif_ipfix_clear(struct dpif_ipfix *di)
OVS_REQUIRES(mutex)
            free(exp_node);
        }

-    HMAP_FOR_EACH_SAFE (dip, next, hmap_node, &di->tunnel_ports) {
-        dpif_ipfix_del_port(di, dip);
+    HMAP_FOR_EACH_SAFE (dip, next, hmap_node, &di->ports) {
+        dpif_ipfix_del_port__(di, dip);
        }
    }

@@ -1162,7 +1208,7 @@ dpif_ipfix_unref(struct dpif_ipfix *di)
OVS_EXCLUDED(mutex)
            dpif_ipfix_clear(di);
            dpif_ipfix_bridge_exporter_destroy(&di->bridge_exporter);
            hmap_destroy(&di->flow_exporter_map);
-        hmap_destroy(&di->tunnel_ports);
+        hmap_destroy(&di->ports);
            free(di);
            ovs_mutex_unlock(&mutex);
        }
@@ -1201,13 +1247,15 @@ ipfix_send_msg(const struct collectors *collectors,
struct dp_packet *msg)

    static uint16_t
    ipfix_get_template_id(enum ipfix_proto_l2 l2, enum ipfix_proto_l3 l3,
-                      enum ipfix_proto_l4 l4, enum ipfix_proto_tunnel tunnel)
+                      enum ipfix_proto_l4 l4, enum ipfix_proto_tunnel tunnel,
+                      enum ipfix_flow_direction flow_direction)
    {
        uint16_t template_id;
        template_id = l2;
        template_id = template_id * NUM_IPFIX_PROTO_L3 + l3;
        template_id = template_id * NUM_IPFIX_PROTO_L4 + l4;
        template_id = template_id * NUM_IPFIX_PROTO_TUNNEL + tunnel;
+    template_id = template_id * NUM_IPFIX_FLOW_DIRECTION +
flow_direction;
        return IPFIX_TEMPLATE_ID_MIN + template_id;
    }

@@ -1219,7 +1267,8 @@ ipfix_get_options_template_id(enum
ipfix_options_template opt_tmpl_type)
        uint16_t max_tmpl_id = ipfix_get_template_id(NUM_IPFIX_PROTO_L2,
                                                     NUM_IPFIX_PROTO_L3,
                                                     NUM_IPFIX_PROTO_L4,
-                                                 NUM_IPFIX_PROTO_TUNNEL);
+                                                 NUM_IPFIX_PROTO_TUNNEL,
+                                                 NUM_IPFIX_FLOW_DIRECTION);

        return max_tmpl_id + opt_tmpl_type;
    }
@@ -1315,7 +1364,9 @@ ipfix_def_options_template_fields(enum
ipfix_options_template opt_tmpl_type,
    static uint16_t
    ipfix_define_template_fields(enum ipfix_proto_l2 l2, enum ipfix_proto_l3 l3,
                                 enum ipfix_proto_l4 l4, enum 
ipfix_proto_tunnel tunnel,
-                             bool virtual_obs_id_set, size_t tmpl_hdr_offset,
+                             enum ipfix_flow_direction flow_direction,
+                             bool virtual_obs_id_set,
+                             size_t tmpl_hdr_offset,
                                 struct dp_packet *msg)
    {

@@ -1333,6 +1384,19 @@ ipfix_define_template_fields(enum ipfix_proto_l2
l2,
enum ipfix_proto_l3 l3,
        DEF(ETHERNET_TYPE);
        DEF(ETHERNET_HEADER_LENGTH);

+    /* Interface Information Elements */
+    DEF(INGRESS_INTERFACE);
+    DEF(INGRESS_INTERFACE_TYPE);
+    DEF(INTERFACE_NAME);
+    DEF(INTERFACE_DESCRIPTION);
+
+    if (flow_direction == EGRESS_FLOW) {
+        DEF(EGRESS_INTERFACE);
+        DEF(EGRESS_INTERFACE_TYPE);
+        DEF(INTERFACE_NAME);
+        DEF(INTERFACE_DESCRIPTION);
+    }
+
        if (l2 == IPFIX_PROTO_L2_VLAN) {
            DEF(VLAN_ID);
            DEF(DOT1Q_VLAN_ID);
@@ -1530,6 +1594,24 @@ ipfix_send_options_template_msgs(struct
dpif_ipfix_exporter *exporter,
    }

    static void
+ipfix_add_template_record(enum ipfix_proto_l2 l2, enum ipfix_proto_l3 l3,
+                          enum ipfix_proto_l4 l4,
+                          enum ipfix_proto_tunnel tunnel,
+                          enum ipfix_flow_direction flow_direction,
+                          bool virtual_obs_id_set,
+                          struct dp_packet *msg)
+{
+    struct ipfix_template_record_header *tmpl_hdr;
+    size_t tmpl_hdr_offset = dp_packet_size(msg);
+
+    tmpl_hdr = dp_packet_put_zeros(msg, sizeof *tmpl_hdr);
+    tmpl_hdr->template_id =
+        htons(ipfix_get_template_id(l2, l3, l4, tunnel, flow_direction));
+    ipfix_define_template_fields(l2, l3, l4, tunnel, flow_direction,
+                                 virtual_obs_id_set, tmpl_hdr_offset, msg);
+}
+
+static void
    ipfix_send_template_msgs(struct dpif_ipfix_exporter *exporter,
                             uint32_t export_time_sec, uint32_t obs_domain_id)
    {
@@ -1537,14 +1619,14 @@ ipfix_send_template_msgs(struct
dpif_ipfix_exporter *exporter,
        struct dp_packet msg;
        dp_packet_use_stub(&msg, msg_stub, sizeof msg_stub);

-    size_t set_hdr_offset, tmpl_hdr_offset, error_pkts;
-    struct ipfix_template_record_header *tmpl_hdr;
+    size_t set_hdr_offset, error_pkts;
        size_t tx_packets = 0;
        size_t tx_errors = 0;
        enum ipfix_proto_l2 l2;
        enum ipfix_proto_l3 l3;
        enum ipfix_proto_l4 l4;
        enum ipfix_proto_tunnel tunnel;
+    enum ipfix_flow_direction flow_direction;

        ipfix_init_template_msg(export_time_sec, exporter->seq_number,
                                obs_domain_id, IPFIX_SET_ID_TEMPLATE, &msg,
@@ -1559,41 +1641,44 @@ ipfix_send_template_msgs(struct
dpif_ipfix_exporter *exporter,
                        continue;
                    }
                    for (tunnel = 0; tunnel < NUM_IPFIX_PROTO_TUNNEL; tunnel++) 
{
-                    /* When the size of the template packet reaches
-                     * MAX_MESSAGE_LEN(1024), send it out.
-                     * And then reinitialize the msg to construct a new
-                     * packet for the following templates.
-                     */
-                    if (dp_packet_size(&msg) >= MAX_MESSAGE_LEN) {
-                        /* Send template message. */
-                        error_pkts = 
ipfix_send_template_msg(exporter->collectors,
-                                                             &msg, 
set_hdr_offset);
-                        tx_errors += error_pkts;
-                        tx_packets += collectors_count(exporter->collectors) - 
error_pkts;
-
-                        /* Reinitialize the template msg. */
-                        ipfix_init_template_msg(export_time_sec,
-                                                exporter->seq_number,
-                                                obs_domain_id,
-                                                IPFIX_SET_ID_TEMPLATE,
-                                                &msg,
-                                                &set_hdr_offset);
+                    for (flow_direction = 0;
+                         flow_direction < NUM_IPFIX_FLOW_DIRECTION;
+                         flow_direction++) {
+                        /* When the size of the template packet reaches
+                         * MAX_MESSAGE_LEN(1024), send it out.
+                         * And then reinitialize the msg to construct a new
+                         * packet for the following templates.
+                         */
+                        if (dp_packet_size(&msg) >= MAX_MESSAGE_LEN) {
+                            /* Send template message. */
+                            error_pkts =
+                                ipfix_send_template_msg(exporter->collectors,
+                                                        &msg, set_hdr_offset);
+                            tx_errors += error_pkts;
+                            tx_packets +=
+                                collectors_count(exporter->collectors)
+                                - error_pkts;
+
+                            /* Reinitialize the template msg. */
+                            ipfix_init_template_msg(export_time_sec,
+                                                    exporter->seq_number,
+                                                    obs_domain_id,
+                                                    IPFIX_SET_ID_TEMPLATE,
+                                                    &msg, &set_hdr_offset);
+                        }
+
+                        ipfix_add_template_record(l2, l3, l4, tunnel,
+                                flow_direction,
+                                exporter->virtual_obs_id != NULL, &msg);
                        }
-
-                    tmpl_hdr_offset = dp_packet_size(&msg);
-                    tmpl_hdr = dp_packet_put_zeros(&msg, sizeof *tmpl_hdr);
-                    tmpl_hdr->template_id = htons(
-                        ipfix_get_template_id(l2, l3, l4, tunnel));
-                    ipfix_define_template_fields(
-                        l2, l3, l4, tunnel, exporter->virtual_obs_id != NULL,
-                        tmpl_hdr_offset, &msg);
                    }
                }
            }
        }

        /* Send template message. */
-    error_pkts = ipfix_send_template_msg(exporter->collectors, &msg,
set_hdr_offset);
+    error_pkts = ipfix_send_template_msg(exporter->collectors, &msg,
+                                         set_hdr_offset);
        tx_errors += error_pkts;
        tx_packets += collectors_count(exporter->collectors) - error_pkts;

@@ -1878,8 +1963,80 @@ ipfix_cache_update(struct dpif_ipfix_exporter
*exporter,
        }
    }

+static void
+ipfix_destroy_iface_data_record(struct ipfix_data_record_flow_key_iface
*data)
+{
+    free(data->if_descr);
+    free(data->if_name);
+}
+
+/* Fills '*data' structure based on port number 'port_no'.  Caller must destroy
+ * 'data' with ipfix_destroy_iface_data_record(). */
+static int
+ipfix_get_iface_data_record(const struct dpif_ipfix *di, odp_port_t port_no,
+                            struct ipfix_data_record_flow_key_iface *data)
+    OVS_REQUIRES(mutex)
+{
+    struct dpif_ipfix_port *port;
+    struct smap netdev_status;
+
+    port = dpif_ipfix_find_port(di, port_no);
+    if (!port) {
+        *data = (struct ipfix_data_record_flow_key_iface) {0};
+        return -1;
+    }
+
+    smap_init(&netdev_status);
+    if (!netdev_get_status(port->ofport->netdev, &netdev_status)) {
+        data->if_type = htonl(smap_get_ullong(&netdev_status, "if_type", 0));
+        data->if_descr = nullable_xstrdup(smap_get(&netdev_status,
+                                                   "if_descr"));
+    } else {
+        data->if_type = 0;
+        data->if_descr = NULL;
+    }
+
+    smap_destroy(&netdev_status);
+    data->if_index = htonl(port->ifindex);
+    data->if_descr_len = data->if_descr ? strlen(data->if_descr) : 0;
+    data->if_name = nullable_xstrdup(netdev_get_name(port->ofport-
netdev));
+    data->if_name_len = data->if_name ? strlen(data->if_name) : 0;
+
+    return 0;
+}
+
+static void
+ipfix_put_iface_data_record(const struct dpif_ipfix *di, odp_port_t port_no,
+                            struct dp_packet *msg)
+    OVS_REQUIRES(mutex)
+{
+    struct ipfix_data_record_flow_key_iface data;
+    int err;
+
+    err = ipfix_get_iface_data_record(di, port_no, &data);
+    if (err == 0) {
+        dp_packet_put(msg, &data.if_index, sizeof data.if_index);
+        dp_packet_put(msg, &data.if_type, sizeof data.if_type);
+        dp_packet_put(msg, &data.if_name_len, sizeof data.if_name_len);
+        if (data.if_name_len) {
+            dp_packet_put(msg, data.if_name, data.if_name_len);
+        }
+        dp_packet_put(msg, &data.if_descr_len, sizeof data.if_descr_len);
+        if (data.if_descr_len) {
+            dp_packet_put(msg, data.if_descr, data.if_descr_len);
+        }
+        ipfix_destroy_iface_data_record(&data);
+    } else {
+        dp_packet_put_zeros(msg, sizeof data.if_index);
+        dp_packet_put_zeros(msg, sizeof data.if_type);
+        dp_packet_put_zeros(msg, sizeof data.if_name_len);
+        dp_packet_put_zeros(msg, sizeof data.if_descr_len);
+    }
+}
+
    static enum ipfix_sampled_packet_type
-ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
+ipfix_cache_entry_init(const struct dpif_ipfix *di,
+                       struct ipfix_flow_cache_entry *entry,
                           const struct dp_packet *packet, const struct flow 
*flow,
                           uint64_t packet_delta_count, uint32_t obs_domain_id,
                           uint32_t obs_point_id, odp_port_t output_odp_port,
@@ -1887,6 +2044,7 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry
*entry,
                           const struct dpif_ipfix_port *tunnel_port,
                           const struct flow_tnl *tunnel_key,
                           struct dpif_ipfix_global_stats *stats)
+    OVS_REQUIRES(mutex)
    {
        struct ipfix_flow_key *flow_key;
        struct dp_packet msg;
@@ -1961,8 +2119,14 @@ ipfix_cache_entry_init(struct
ipfix_flow_cache_entry
*entry,
           tunnel = IPFIX_PROTO_TUNNELED;
        }

+    uint8_t flow_direction =
+        (direction == NX_ACTION_SAMPLE_INGRESS ? INGRESS_FLOW
+         : direction == NX_ACTION_SAMPLE_EGRESS ? EGRESS_FLOW
+         : output_odp_port == ODPP_NONE ? INGRESS_FLOW : EGRESS_FLOW);
+
        flow_key->obs_domain_id = obs_domain_id;
-    flow_key->template_id = ipfix_get_template_id(l2, l3, l4, tunnel);
+    flow_key->template_id = ipfix_get_template_id(l2, l3, l4, tunnel,
+                                                  flow_direction);

        /* The fields defined in the ipfix_data_record_* structs and sent
         * below must match exactly the templates defined in
@@ -1972,11 +2136,6 @@ ipfix_cache_entry_init(struct
ipfix_flow_cache_entry
*entry,
            ? VLAN_ETH_HEADER_LEN : ETH_HEADER_LEN;
        ethernet_total_length = dp_packet_size(packet);

-    uint8_t flow_direction =
-        (direction == NX_ACTION_SAMPLE_INGRESS ? INGRESS_FLOW
-         : direction == NX_ACTION_SAMPLE_EGRESS ? EGRESS_FLOW
-         : output_odp_port == ODPP_NONE ? INGRESS_FLOW : EGRESS_FLOW);
-
        /* Common Ethernet entities. */
        {
            struct ipfix_data_record_flow_key_common *data_common;
@@ -1990,6 +2149,13 @@ ipfix_cache_entry_init(struct
ipfix_flow_cache_entry
*entry,
            data_common->ethernet_header_length = ethernet_header_length;
        }

+    /* Interface Information Elements */
+    ipfix_put_iface_data_record(di, flow->in_port.odp_port, &msg);
+
+    if (flow_direction == EGRESS_FLOW) {
+        ipfix_put_iface_data_record(di, output_odp_port, &msg);
+    }
+
        if (l2 == IPFIX_PROTO_L2_VLAN) {
            struct ipfix_data_record_flow_key_vlan *data_vlan;
            uint16_t vlan_id = vlan_tci_to_vid(flow->vlans[0].tci);
@@ -2414,13 +2580,15 @@ ipfix_send_data_msg(struct dpif_ipfix_exporter
*exporter,
    }

    static void
-dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
+dpif_ipfix_sample(const struct dpif_ipfix *di,
+                  struct dpif_ipfix_exporter *exporter,
                      const struct dp_packet *packet, const struct flow *flow,
                      uint64_t packet_delta_count, uint32_t obs_domain_id,
                      uint32_t obs_point_id, odp_port_t output_odp_port,
                      enum nx_action_sample_direction direction,
                      const struct dpif_ipfix_port *tunnel_port,
                      const struct flow_tnl *tunnel_key)
+    OVS_REQUIRES(mutex)
    {
        struct ipfix_flow_cache_entry *entry;
        enum ipfix_sampled_packet_type sampled_packet_type;
@@ -2428,7 +2596,7 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter
*exporter,
        /* Create a flow cache entry from the sample. */
        entry = xmalloc(sizeof *entry);
        sampled_packet_type =
-            ipfix_cache_entry_init(entry, packet,
+            ipfix_cache_entry_init(di, entry, packet,
                                       flow, packet_delta_count,
                                       obs_domain_id, obs_point_id,
                                       output_odp_port, direction,
@@ -2484,16 +2652,16 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di,
const
struct dp_packet *packet,
            if (output_odp_port == ODPP_NONE && flow->tunnel.ip_dst) {
                /* Input tunnel. */
                tunnel_key = &flow->tunnel;
-            tunnel_port = dpif_ipfix_find_port(di, input_odp_port);
+            tunnel_port = dpif_ipfix_find_tunnel_port(di, input_odp_port);
            }
            if (output_odp_port != ODPP_NONE && output_tunnel_key) {
                /* Output tunnel, output_tunnel_key must be valid. */
                tunnel_key = output_tunnel_key;
-            tunnel_port = dpif_ipfix_find_port(di, output_odp_port);
+            tunnel_port = dpif_ipfix_find_tunnel_port(di, output_odp_port);
            }
        }

-    dpif_ipfix_sample(&di->bridge_exporter.exporter, packet, flow,
+    dpif_ipfix_sample(di, &di->bridge_exporter.exporter, packet, flow,
                          packet_delta_count,
                          di->bridge_exporter.options->obs_domain_id,
                          di->bridge_exporter.options->obs_point_id,
@@ -2528,16 +2696,16 @@ dpif_ipfix_flow_sample(struct dpif_ipfix *di, const
struct dp_packet *packet,
                if (output_odp_port == ODPP_NONE && flow->tunnel.ip_dst) {
                    /* Input tunnel. */
                    tunnel_key = &flow->tunnel;
-                tunnel_port = dpif_ipfix_find_port(di, input_odp_port);
+                tunnel_port = dpif_ipfix_find_tunnel_port(di, input_odp_port);
                }
                if (output_odp_port != ODPP_NONE && output_tunnel_key) {
                    /* Output tunnel, output_tunnel_key must be valid. */
                    tunnel_key = output_tunnel_key;
-                tunnel_port = dpif_ipfix_find_port(di, output_odp_port);
+                tunnel_port = dpif_ipfix_find_tunnel_port(di, output_odp_port);
                }
            }

-        dpif_ipfix_sample(&node->exporter.exporter, packet, flow,
+        dpif_ipfix_sample(di, &node->exporter.exporter, packet, flow,
                              packet_delta_count,
                              cookie->flow_sample.obs_domain_id,
                              cookie->flow_sample.obs_point_id,
diff --git a/ofproto/ofproto-dpif-ipfix.h b/ofproto/ofproto-dpif-ipfix.h
index 0808ede..0b21441 100644
--- a/ofproto/ofproto-dpif-ipfix.h
+++ b/ofproto/ofproto-dpif-ipfix.h
@@ -33,8 +33,8 @@ struct dpif_ipfix *dpif_ipfix_create(void);
    struct dpif_ipfix *dpif_ipfix_ref(const struct dpif_ipfix *);
    void dpif_ipfix_unref(struct dpif_ipfix *);

-void dpif_ipfix_add_tunnel_port(struct dpif_ipfix *, struct ofport *,
odp_port_t);
-void dpif_ipfix_del_tunnel_port(struct dpif_ipfix *, odp_port_t);
+void dpif_ipfix_add_port(struct dpif_ipfix *, struct ofport *, odp_port_t);
+void dpif_ipfix_del_port(struct dpif_ipfix *, odp_port_t);

    uint32_t dpif_ipfix_get_bridge_exporter_probability(const struct dpif_ipfix 
*);
    bool dpif_ipfix_get_bridge_exporter_tunnel_sampling(const struct dpif_ipfix
*);
@@ -42,7 +42,7 @@ bool
dpif_ipfix_get_bridge_exporter_input_sampling(const
struct dpif_ipfix *);
    bool dpif_ipfix_get_bridge_exporter_output_sampling(const struct dpif_ipfix
*);
    bool dpif_ipfix_get_flow_exporter_tunnel_sampling(const struct dpif_ipfix *,
                                                      const uint32_t);
-bool dpif_ipfix_get_tunnel_port(const struct dpif_ipfix *, odp_port_t);
+bool dpif_ipfix_is_tunnel_port(const struct dpif_ipfix *, odp_port_t);
    void dpif_ipfix_set_options(
        struct dpif_ipfix *,
        const struct ofproto_ipfix_bridge_exporter_options *,
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7f7adb2..3a37559 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2946,7 +2946,7 @@ compose_ipfix_action(struct xlate_ctx *ctx,
odp_port_t
output_odp_port)
             * OVS_USERSPACE_ATTR_TUNNEL_OUT_PORT
             */
            if (dpif_ipfix_get_bridge_exporter_tunnel_sampling(ipfix) &&
-            dpif_ipfix_get_tunnel_port(ipfix, output_odp_port) ) {
+            dpif_ipfix_is_tunnel_port(ipfix, output_odp_port) ) {
               tunnel_out_port = output_odp_port;
            }
        }
@@ -5199,7 +5199,7 @@ xlate_sample_action(struct xlate_ctx *ctx,

            if (dpif_ipfix_get_flow_exporter_tunnel_sampling(ipfix,
                                                             
os->collector_set_id)
-            && dpif_ipfix_get_tunnel_port(ipfix, output_odp_port)) {
+            && dpif_ipfix_is_tunnel_port(ipfix, output_odp_port)) {
                tunnel_out_port = output_odp_port;
                emit_set_tunnel = true;
            }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 50f440f..72a4334 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1865,9 +1865,6 @@ port_construct(struct ofport *port_)
            }

            port->is_tunnel = true;
-        if (ofproto->ipfix) {
-           dpif_ipfix_add_tunnel_port(ofproto->ipfix, port_, port->odp_port);
-        }
        } else {
            /* Sanity-check that a mapping doesn't already exist.  This
             * shouldn't happen for non-tunnel ports. */
@@ -1888,6 +1885,9 @@ port_construct(struct ofport *port_)
        if (ofproto->sflow) {
            dpif_sflow_add_port(ofproto->sflow, port_, port->odp_port);
        }
+    if (ofproto->ipfix) {
+       dpif_ipfix_add_port(ofproto->ipfix, port_, port->odp_port);
+    }

        return 0;
    }
@@ -1933,10 +1933,6 @@ port_destruct(struct ofport *port_, bool del)
            atomic_count_dec(&ofproto->backer->tnl_count);
        }

-    if (port->is_tunnel && ofproto->ipfix) {
-       dpif_ipfix_del_tunnel_port(ofproto->ipfix, port->odp_port);
-    }
-
        tnl_port_del(port);
        sset_find_and_delete(&ofproto->ports, devname);
        sset_find_and_delete(&ofproto->ghost_ports, devname);
@@ -1951,6 +1947,9 @@ port_destruct(struct ofport *port_, bool del)
        if (ofproto->sflow) {
            dpif_sflow_del_port(ofproto->sflow, port->odp_port);
        }
+    if (ofproto->ipfix) {
+       dpif_ipfix_del_port(ofproto->ipfix, port->odp_port);
+    }

        free(port->qdscp);
    }
@@ -2076,13 +2075,11 @@ set_ipfix(
                di, bridge_exporter_options, flow_exporters_options,
                n_flow_exporters_options);

-        /* Add tunnel ports only when a new ipfix created */
+        /* Add ports only when a new ipfix created */
            if (new_di == true) {
                struct ofport_dpif *ofport;
                HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
-                if (ofport->is_tunnel == true) {
-                    dpif_ipfix_add_tunnel_port(di, &ofport->up, 
ofport->odp_port);
-                }
+                dpif_ipfix_add_port(di, &ofport->up, ofport->odp_port);
                }
            }

--
1.8.3.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to