Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-18 Thread Greg Rose

On 09/18/2017 03:01 AM, Weglicki, MichalX wrote:

Hi Greg - comments inline marked [MW].


-Original Message-
From: Greg Rose [mailto:gvrose8...@gmail.com]
Sent: Saturday, September 16, 2017 12:45 AM
To: Weglicki, MichalX <michalx.wegli...@intel.com>
Cc: d...@openvswitch.org; Darrell Ball <db...@vmware.com>
Subject: Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface 
Information Elements to flow key

On 09/08/2017 06:35 AM, Weglicki, MichalX wrote:

Greg,

Patch is rebased and sent to mailing list as V3 (Last patch was supposed to be 
V2 - Przemek by accident sent it again as V1).

Br,
Michal.


Michal,

I'm not sure what has happened but I can't find your V3 patches in my mail but 
they are in patchwork.

I tested the patches and they seem to work fine, or at least the code executes 
and I didn't see any serious
regression.  My ntopng/nprobe setup accepted the new template.

However, I am seeing this now:

5/Sep/2017 15:11:16 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : expected 
string, got nil
15/Sep/2017 15:11:16 [Lua.cpp:6658] WARNING: Script failure
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31:
 attempt to index
global
'stats' (a nil value)]
15/Sep/2017 15:11:16 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:15 [Lua.cpp:6658] WARNING: Script failure
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31:
 attempt to index
global
'stats' (a nil value)]
15/Sep/2017 15:11:15 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:12 [Lua.cpp:6658] WARNING: Script failure
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31:
 attempt to index
global
'stats' (a nil value)]
15/Sep/2017 15:11:12 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:09 [Lua.cpp:6658] WARNING: Script failure
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31:
 attempt to index
global
'stats' (a nil value)]
15/Sep/2017 15:11:09 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:09 [Lua.cpp:6658] WARNING: Script failure
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31:
 attempt to index
global
'stats' (a nil value)]
15/Sep/2017 15:11:09 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil

[MW] Well, are you sure that this patch is causing this error message? I mean, 
you don't see
It without this patch, do you? If yes, could you explain your setup a little 
bit so I can reproduce it here?


It took a while but I was able to reproduce without your patches applied.  
Seems to be something that
occasionally occurs soon after the ntopng/nprobe services are restarted and 
then it doesn't occur
again SFAICT.






Also, in patch 1/2 though the interface is hard coded to Ethernet in this bit 
of the patch:

+/* Once DPDK library supports retrieving ifType we should get this value
+ * directly from DPDK rather than hardcoding it. */
+smap_add_format(args, "if_type", "%"PRIu32, IF_TYPE_ETHERNETCSMACD);
+smap_add_format(args, "if_descr", "%s %s", rte_version(),
+   dev_info.driver_name);

I'd like to get Darrel Ball's take on this so I've CC'd him.

In patch 2/2 I have some other comments.

[MW] There are some DPDK patches which expose this information for the driver, 
nevertheless I think that anyway value "6"
Will be returned anyway - as this is very general category for Ethernet 
devices. I could be improved in the future of course
When patch will be available.



Here you change the ports to point to all ports rather than tunnel ports

-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;

I didn't see any reason this can't be done but I'm worried about side effects. 
Are we sure that
there are no other assumptions in the code that depend on that hmap pointing to 
only tunnel ports?
I realize that patch 2/2 seems to fix that up and I didn't see any particular 
reason to
doubt but I'm rather new to the code base so I thought I'd ask.

[MW] I'm not quite sure wat do you mean here, in previous implementation only 
tunnel ports were
Mandatory to cache, however currently we need all ports, as we need to query 
particular
Netdev to get information that we need. I don't remember exactly the detai

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-15 Thread Greg Rose

On 09/08/2017 06:35 AM, Weglicki, MichalX wrote:

Greg,

Patch is rebased and sent to mailing list as V3 (Last patch was supposed to be 
V2 - Przemek by accident sent it again as V1).

Br,
Michal.


Michal,

I'm not sure what has happened but I can't find your V3 patches in my mail but 
they are in patchwork.

I tested the patches and they seem to work fine, or at least the code executes 
and I didn't see any serious
regression.  My ntopng/nprobe setup accepted the new template.

However, I am seeing this now:

5/Sep/2017 15:11:16 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : expected 
string, got nil
15/Sep/2017 15:11:16 [Lua.cpp:6658] WARNING: Script failure 
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31: attempt to index global 
'stats' (a nil value)]

15/Sep/2017 15:11:16 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:15 [Lua.cpp:6658] WARNING: Script failure 
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31: attempt to index global 
'stats' (a nil value)]

15/Sep/2017 15:11:15 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:12 [Lua.cpp:6658] WARNING: Script failure 
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31: attempt to index global 
'stats' (a nil value)]

15/Sep/2017 15:11:12 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:09 [Lua.cpp:6658] WARNING: Script failure 
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31: attempt to index global 
'stats' (a nil value)]

15/Sep/2017 15:11:09 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil
15/Sep/2017 15:11:09 [Lua.cpp:6658] WARNING: Script failure 
[/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua][/usr/share/ntopng/scripts/lua/iface_ndpi_stats.lua:31: attempt to index global 
'stats' (a nil value)]

15/Sep/2017 15:11:09 [Lua.cpp:74] ERROR: ntop_get_interface_host_info : 
expected string, got nil

Also, in patch 1/2 though the interface is hard coded to Ethernet in this bit 
of the patch:

+/* Once DPDK library supports retrieving ifType we should get this value
+ * directly from DPDK rather than hardcoding it. */
+smap_add_format(args, "if_type", "%"PRIu32, IF_TYPE_ETHERNETCSMACD);
+smap_add_format(args, "if_descr", "%s %s", rte_version(),
+   dev_info.driver_name);

I'd like to get Darrel Ball's take on this so I've CC'd him.

In patch 2/2 I have some other comments.

Here you change the ports to point to all ports rather than tunnel ports

-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;

I didn't see any reason this can't be done but I'm worried about side effects. 
Are we sure that
there are no other assumptions in the code that depend on that hmap pointing to 
only tunnel ports?
I realize that patch 2/2 seems to fix that up and I didn't see any particular 
reason to
doubt but I'm rather new to the code base so I thought I'd ask.

Further on I see this:

+static enum dpif_ipfix_tunnel_type
+dpif_ipfix_tunnel_type(const struct ofport *ofport) {

I think the opening curly brace should be on the next line down?  I saw at 
least one other instance
if this.  It passes checkpatch to put the curly brace on the same line but all 
other functions in
the source module put the opening curly brace of a function on the next line.

There are a couple of other checkpatch warnings and an error as well:

ERROR: Too many signoffs; are you missing Co-authored-by lines?
WARNING: Line lacks whitespace around operator
#80 FILE: ofproto/ofproto-dpif-ipfix.c:317:
ovs_be32 if_index; /* (INGRESS|EGRESS)_INTERFACE */

WARNING: Line lacks whitespace around operator
#81 FILE: ofproto/ofproto-dpif-ipfix.c:318:
ovs_be32 if_type; /* (INGRESS|EGRESS)_INTERFACE_TYPE */

Lines checked: 762, Warnings: 2, Errors: 1

The rest of patch 2/2 looks OK but I am worried about the ntop errors listed 
above.
I haven't seen those before so I think they're related to this patch.

Thanks for work on this!  Hopefully we can sort this all out and get your ipfix
feature enhancement applied.

Regards,

- Greg


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


Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-08 Thread Greg Rose

On 09/08/2017 06:35 AM, Weglicki, MichalX wrote:

Greg,

Patch is rebased and sent to mailing list as V3 (Last patch was supposed to be 
V2 - Przemek by accident sent it again as V1).

Br,
Michal.


Thanks, I'll have a look at it!

- Greg



> -Original Message-
> From: Greg Rose [mailto:gvrose8...@gmail.com]
> Sent: Thursday, September 7, 2017 12:02 AM
> 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 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.
>
> Michal,
>
> The good news is that I have my ntopng/nprobe ipfix collector properly 
configured now and can see
> flows, hosts, etc. when I enable IPFIX.  So I'm ready to test these two 
patches from Przemyslaw:
>
> https://patchwork.ozlabs.org/patch/793773/
> https://patchwork.ozlabs.org/patch/793772/
>
> Unfortunately the patches no longer apply to master.
>
> Can you rebase and resubmit a V3 series?
>
> 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
>>>

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-08 Thread Weglicki, MichalX
Greg, 

Patch is rebased and sent to mailing list as V3 (Last patch was supposed to be 
V2 - Przemek by accident sent it again as V1). 

Br, 
Michal. 

> -Original Message-
> From: Greg Rose [mailto:gvrose8...@gmail.com]
> Sent: Thursday, September 7, 2017 12:02 AM
> 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 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.
> 
> Michal,
> 
> The good news is that I have my ntopng/nprobe ipfix collector properly 
> configured now and can see
> flows, hosts, etc. when I enable IPFIX.  So I'm ready to test these two 
> patches from Przemyslaw:
> 
> https://patchwork.ozlabs.org/patch/793773/
> https://patchwork.ozlabs.org/patch/793772/
> 
> Unfortunately the patches no longer apply to master.
> 
> Can you rebase and resubmit a V3 series?
> 
> 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
> >>>> udp0  0 0.0.0.0

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-06 Thread Greg Rose

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.


Michal,

The good news is that I have my ntopng/nprobe ipfix collector properly 
configured now and can see
flows, hosts, etc. when I enable IPFIX.  So I'm ready to test these two patches 
from Przemyslaw:

https://patchwork.ozlabs.org/patch/793773/
https://patchwork.ozlabs.org/patch/793772/

Unfortunately the patches no longer apply to master.

Can you rebase and resubmit a V3 series?

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
udp0  0 0.0.0.0:20550.0.0.0:*   
99 27671  3038/nprobe
udp6   0  0 :::2055 :::*
99 27672  3038/nprobe

netstat -tulpen | grep 5556
tcp0  0 0.0.0.0:55560.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 

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-06 Thread Greg Rose

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
udp0  0 0.0.0.0:20550.0.0.0:*   
99 27671  3038/nprobe
udp6   0  0 :::2055 :::*
99 27672  3038/nprobe

netstat -tulpen | grep 5556
tcp0  0 0.0.0.0:55560.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,

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-09-06 Thread Weglicki, MichalX
Hey Greg, 

Do you have any schedule for checking this patch? 

Thank you in advance!

Br, 
Michal. 

> -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
> > > udp0  0 0.0.0.0:20550.0.0.0:* 
> > >   99 27671  3038/nprobe
> > > udp6   0  0 :::2055 :::*  
> > >   99 27672  3038/nprobe
> > >
> > > netstat -tulpen | grep 5556
> > > tcp0  0 0.0.0.0:55560.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
> > >

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-08-29 Thread Greg Rose

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
> udp0  0 0.0.0.0:20550.0.0.0:* 
  99 27671  3038/nprobe
> udp6   0  0 :::2055 :::*  
  99 27672  3038/nprobe
>
> netstat -tulpen | grep 5556
> tcp0  0 0.0.0.0:55560.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 interfa

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-08-29 Thread Weglicki, MichalX
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. 

> -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
> udp0  0 0.0.0.0:20550.0.0.0:* 
>   99 27671  3038/nprobe
> udp6   0  0 :::2055 :::*  
>   99 27672  3038/nprobe
> 
> netstat -tulpen | grep 5556
> tcp0  0 0.0.0.0:55560.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
> > >>
> > >>

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-08-18 Thread Greg Rose

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
udp0  0 0.0.0.0:20550.0.0.0:*   
99 27671  3038/nprobe
udp6   0  0 :::2055 :::*
99 27672  3038/nprobe

netstat -tulpen | grep 5556
tcp0  0 0.0.0.0:55560.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 

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-08-17 Thread Greg Rose

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



[snip]

I'll have a look at it over the next few days and see if I can provide some 
feedback.  I was investigating other issues with ipfix some time back but got 
pulled into some other duties.  Hopefully I can get it working and provide some 
feedback.

Thanks,

- Greg

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


Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-08-16 Thread Szczerbik, PrzemyslawX
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

> -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
> > + * 

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-07-26 Thread Szczerbik, PrzemyslawX
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 
> 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 
> ---
> 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),
> - >tunnel_ports) {
> + >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(>tunnel_ports, >hmap_node);
> +hmap_remove(>ports, >hmap_node);
>  free(dip);
>  }
> 
> 

[ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key

2017-07-26 Thread Przemyslaw Szczerbik
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 
---
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),
- >tunnel_ports) {
+ >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(>tunnel_ports, >hmap_node);
+hmap_remove(>ports, >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