Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-ipfix: add interface Information Elements to flow key
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
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
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
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
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
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
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
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
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
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
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
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
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
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