[ovs-dev] test
The message was undeliverable due to the following reason(s): Your message was not delivered because the destination server was unreachable within the allowed queue period. The amount of time a message is queued before it is returned depends on local configura- tion parameters. Most likely there is a network problem that prevented delivery, but it is also possible that the computer is turned off, or does not have a mail system running right now. Your message was not delivered within 6 days: Host 212.229.173.154 is not responding. The following recipients could not receive this message: Please reply to postmas...@openvswitch.org if you feel this message to be in error. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] STATUS
The original message was included as attachment ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] datapath: Use alternate name for udp_sock_create() backport
Account for kernels which provide udp_sock_create() in an insufficient version. Avoids the following error when inserting openvswitch.ko on respective kernels: openvswitch: exports duplicate symbol udp_sock_create (owned by udp_tunnel) Fixes: eb6eebd28 ("datapath: Account for "udp: Add udp_sock_create for UDP tunnels to open listener socket") Signed-off-by: Thomas Graf Cc: Jesse Gross --- datapath/linux/compat/include/net/udp_tunnel.h | 1 + 1 file changed, 1 insertion(+) diff --git a/datapath/linux/compat/include/net/udp_tunnel.h b/datapath/linux/compat/include/net/udp_tunnel.h index 7dadac1..6c25ca5 100644 --- a/datapath/linux/compat/include/net/udp_tunnel.h +++ b/datapath/linux/compat/include/net/udp_tunnel.h @@ -49,6 +49,7 @@ struct udp_port_cfg { use_udp6_rx_checksums:1; }; +#define udp_sock_create rpl_udp_sock_create int udp_sock_create(struct net *net, struct udp_port_cfg *cfg, struct socket **sockp); -- 1.9.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Turn vports with dependencies into separate modules
On 03/25/15 at 02:48pm, Pravin Shelar wrote: > Thanks for the patch. I saw couple of issues:- > I saw following warning due to missing symbol. > WARNING: "lockdep_ovsl_is_held" > [/home/pravin/ovs/w8/datapath/linux/vport-gre.ko] undefined! Thanks for catching! Fixed. > Command to reload kernel datapath does not work anymore. > `ovs-ctl force-reload-kmod` seg faults as follows: > > > device br0 left promiscuous mode > general protection fault: [#1] SMP DEBUG_PAGEALLOC > Modules linked in: openvswitch(OF) vxlan ip_tunnel libcrc32c veth > netconsole configfs autofs4 ipt_REJECT ip6t_REJECT nf_conntrack_ipv6 > nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 > vhost_net macvtap macvlan vhost tun kvm_intel kvm iTCO_wdt > iTCO_vendor_support dcdbas microcode pcspkr sb_edac edac_core acpi_pad > lpc_ich mfd_core shpchp tg3 ptp pps_core ses enclosure sg wmi ext4(F) > jbd2(F) mbcache(F) usb_storage(F) sd_mod(F) crc_t10dif(F) > crct10dif_common(F) megaraid_sas(F) dm_mirror(F) dm_region_hash(F) > dm_log(F) dm_mod(F) [last unloaded: openvswitch] > CPU: 10 PID: 22434 Comm: ovs-dpctl Tainted: GF O 3.14.30 #4 > Hardware name: Dell Inc. PowerEdge T320/07C9XP, BIOS 2.1.2 01/20/2014 > task: 8807e756 ti: 8807fd7e6000 task.ti: 8807fd7e6000 > RIP: 0010:[] [] > ovs_vport_del+0x4a/0x70 [openvswitch] Tracked down to upstream bug. Will submit proper upstream fix and include it in the v2 of this backport. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net] openvswitch: Return vport module ref before destruction
Return module reference before invoking the respective vport ->destroy() function. This is needed as ovs_vport_del() is not invoked inside an RCU read side critical section so the kfree can occur immediately before returning to ovs_vport_del(). Returning the module reference before ->destroy() is safe because the module unregistration is blocked on ovs_lock which we hold while destroying the datapath. Fixes: 62b9c8d0372d ("ovs: Turn vports with dependencies into separate modules") Reported-by: Pravin Shelar Signed-off-by: Thomas Graf --- @Dave: Please also queue for 3.19.x stable series net/openvswitch/vport.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c index ec2954f..067a3ff 100644 --- a/net/openvswitch/vport.c +++ b/net/openvswitch/vport.c @@ -274,10 +274,8 @@ void ovs_vport_del(struct vport *vport) ASSERT_OVSL(); hlist_del_rcu(&vport->hash_node); - - vport->ops->destroy(vport); - module_put(vport->ops->owner); + vport->ops->destroy(vport); } /** -- 1.9.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Finance Entre Particulier
Bonjour Mlle, Mme et Mr. Vous aviez besoin d’un financement, d’un prêt immobilière, vous êtes interdit bancaire, fiché, vous aviez quelques soucis financières pour monter vos projets? Je suis un particulier financier octroyant des prêts allant de 3.000 à 600.000 € à des conditions abordable,si vous êtes dans le besoin n’hésiter pas à me joindre pour de plus d'informations. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Finance Entre Particulier
Bonjour Mlle, Mme et Mr. Vous aviez besoin d’un financement, d’un prêt immobilière, vous êtes interdit bancaire, fiché, vous aviez quelques soucis financières pour monter vos projets? Je suis un particulier financier octroyant des prêts allant de 3.000 à 600.000 € à des conditions abordable,si vous êtes dans le besoin n’hésiter pas à me joindre pour de plus d'informations. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovs-ctl: Unload vport modules on force-reload-kmod
We manually rmmod the loaded vports as using modprobe -r only works if the modules are available through modules.dep Upon restart, the vports get automatically reloaded when the respective vports are added. Reported-by: Pravin Shelar Signed-off-by: Thomas Graf --- utilities/ovs-ctl.in | 4 1 file changed, 4 insertions(+) diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index 6d2e938..c16b4e0 100755 --- a/utilities/ovs-ctl.in +++ b/utilities/ovs-ctl.in @@ -388,6 +388,10 @@ force_reload_kmod () { action "Removing datapath: $dp" ovs-dpctl del-dp "$dp" done +for vport in `lsmod | grep ^vport_ | awk '{print $1;}'`; do +action "Removing $vport module" rmmod $vport +done + # try both old and new names in case this is post upgrade if test -e /sys/module/openvswitch_mod; then action "Removing openvswitch module" rmmod openvswitch_mod -- 1.9.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] build-aux/cccl: Enhance --with-debug option
This patch changes the behaviour in case the configure argument: --with-debug was specified. Currently the optimization flag in the case of debugging is the following: https://msdn.microsoft.com/en-us/library/f9534wye.aspx which does not fully disable optimization, that is why it was changed with the following flag: https://msdn.microsoft.com/en-us/library/aafb762y.aspx which disables all code optimization. Also this patch includes the definition of the following preprocessor definitions: _DEBUG - in case --with-debug is specified NDEBUG - in case --with-debug was not specified The above definitions usually are defined when compiling with the following flags: https://msdn.microsoft.com/en-us/library/2kzt1wy3.aspx Since we are not compiling with the above flags mimic the behaviour. Signed-off-by: Alin Gabriel Serdean --- build-aux/cccl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-aux/cccl b/build-aux/cccl index 2a4dccc..5f30dd8 100644 --- a/build-aux/cccl +++ b/build-aux/cccl @@ -90,11 +90,11 @@ EOF ;; -O0) -clopt="$clopt ${slash}Ot" +clopt="$clopt ${slash}Od ${slash}D_DEBUG" ;; -O2) -clopt="$clopt ${slash}O2" +clopt="$clopt ${slash}O2 ${slash}DNDEBUG" ;; -L*) -- 1.9.5.msysgit.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] build-aux/cccl: Enhance --with-debug option
This patch changes the behaviour in case the configure argument: --with-debug was specified. Currently the optimization flag in the case of debugging is the following: https://msdn.microsoft.com/en-us/library/f9534wye.aspx which does not fully disable optimization, that is why it was changed with the following flag: https://msdn.microsoft.com/en-us/library/aafb762y.aspx which disables all code optimization. Also this patch includes the definition of the following preprocessor definitions: _DEBUG - in case --with-debug is specified The above definitions usually are defined when compiling with the following flags: https://msdn.microsoft.com/en-us/library/2kzt1wy3.aspx Since we are not compiling with the above flag, mimic the behaviour the debug becahviour. Signed-off-by: Alin Gabriel Serdean --- build-aux/cccl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-aux/cccl b/build-aux/cccl index 2a4dccc..456292e 100644 --- a/build-aux/cccl +++ b/build-aux/cccl @@ -90,7 +90,7 @@ EOF ;; -O0) -clopt="$clopt ${slash}Ot" +clopt="$clopt ${slash}Od ${slash}D_DEBUG" ;; -O2) -- 1.9.5.msysgit.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Use alternate name for udp_sock_create() backport
On Mon, Mar 30, 2015 at 3:27 AM, Thomas Graf wrote: > Account for kernels which provide udp_sock_create() in an > insufficient version. > > Avoids the following error when inserting openvswitch.ko on > respective kernels: > > openvswitch: exports duplicate symbol udp_sock_create (owned by udp_tunnel) > > Fixes: eb6eebd28 ("datapath: Account for "udp: Add udp_sock_create for UDP > tunnels to open listener socket") > Signed-off-by: Thomas Graf > Cc: Jesse Gross Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vxlan: fix kernel crash with vxlan gso
On Sun, Mar 29, 2015 at 9:27 PM, yinpeijun wrote: > From: caochengrong > > tcp flows with gso between two VMs in diffrent host, > go through vxlan tunnel, cause kernel crash. > > Signed-off-by: caochengrong > Signed-off-by: Arika Chen What OVS and host kernel version is this for? I don't think this should be necessary, at least on master. vxlan_xmit_skb() calls udp_tunnel_handle_offloads(), which calls ip_tunnel_handle_offloads(), which calls skb_reset_inner_headers(). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On Thu, Mar 19, 2015 at 07:31:06AM -0700, Gurucharan Shetty wrote: > The design was come up after inputs and discussions with multiple > people, including (in alphabetical order) Aaron Rosen, Ben Pfaff, > Ganesan Chandrashekhar, Justin Pettit, Russell Bryant and Somik Behera. > > Signed-off-by: Gurucharan Shetty I apologize that it took me so long to review this change. Other than the one suggestion I sent in a separate email, Acked-by: Ben Pfaff Thank you so much for being proactive about figuring out how containers fit into the system! It ought to save us a lot of trouble and redesign later. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net] openvswitch: Return vport module ref before destruction
On Mon, Mar 30, 2015 at 4:57 AM, Thomas Graf wrote: > Return module reference before invoking the respective vport > ->destroy() function. This is needed as ovs_vport_del() is not > invoked inside an RCU read side critical section so the kfree > can occur immediately before returning to ovs_vport_del(). > > Returning the module reference before ->destroy() is safe because > the module unregistration is blocked on ovs_lock which we hold > while destroying the datapath. > > Fixes: 62b9c8d0372d ("ovs: Turn vports with dependencies into separate > modules") > Reported-by: Pravin Shelar > Signed-off-by: Thomas Graf looks good. Acked-by: Pravin B Shelar ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On 03/30/2015 01:45 PM, Ben Pfaff wrote: > On Thu, Mar 19, 2015 at 07:31:06AM -0700, Gurucharan Shetty wrote: >> The design was come up after inputs and discussions with multiple >> people, including (in alphabetical order) Aaron Rosen, Ben Pfaff, >> Ganesan Chandrashekhar, Justin Pettit, Russell Bryant and Somik Behera. >> >> Signed-off-by: Gurucharan Shetty > > I apologize that it took me so long to review this change. > > Other than the one suggestion I sent in a separate email, > Acked-by: Ben Pfaff FWIW, same here. I like it. Thanks! Acked-by: Russell Bryant > Thank you so much for being proactive about figuring out how containers > fit into the system! It ought to save us a lot of trouble and redesign > later. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
>> Other than the one suggestion I sent in a separate email, >> Acked-by: Ben Pfaff > > FWIW, same here. I like it. Thanks! > > Acked-by: Russell Bryant > >> Thank you so much for being proactive about figuring out how containers >> fit into the system! It ought to save us a lot of trouble and redesign >> later. Thank you both Russell and Ben. I made the changes that you both recommended and pushed this to the ovn branch. I think it is a good starting point! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On Mon, Mar 30, 2015 at 11:31 AM, Gurucharan Shetty wrote: >>> Other than the one suggestion I sent in a separate email, >>> Acked-by: Ben Pfaff >> >> FWIW, same here. I like it. Thanks! >> >> Acked-by: Russell Bryant >> >>> Thank you so much for being proactive about figuring out how containers >>> fit into the system! It ought to save us a lot of trouble and redesign >>> later. > > > Thank you both Russell and Ben. I made the changes that you both > recommended and pushed this to the ovn branch. I think it is a good > starting point! I think because of the schema changes made because of this commit, it will likely break the ovn-nbctl. This problem will not occur after Ben adds his tests this week. I am happy to fix them (if Russell already hasn't thought about it and has a patch) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Design and Schema changes for Container integration.
On 03/30/2015 02:39 PM, Gurucharan Shetty wrote: > On Mon, Mar 30, 2015 at 11:31 AM, Gurucharan Shetty > wrote: Other than the one suggestion I sent in a separate email, Acked-by: Ben Pfaff >>> >>> FWIW, same here. I like it. Thanks! >>> >>> Acked-by: Russell Bryant >>> Thank you so much for being proactive about figuring out how containers fit into the system! It ought to save us a lot of trouble and redesign later. >> >> >> Thank you both Russell and Ben. I made the changes that you both >> recommended and pushed this to the ovn branch. I think it is a good >> starting point! > I think because of the schema changes made because of this commit, it > will likely break the ovn-nbctl. This problem will not occur after Ben > adds his tests this week. I am happy to fix them (if Russell already > hasn't thought about it and has a patch) It doesn't seem to be broken. I just rebuilt it and ran ovs-sandbox and it still seems to be working the same. Here's the simple manual test I used: https://gist.github.com/russellb/946953e8675063c0c756 -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCHv4 1/9] dpif-netdev: Account for and free lost packets.
Acked-by: Ethan Jackson Merged, thanks. On Fri, Mar 27, 2015 at 9:29 AM, Daniele Di Proietto wrote: > Packets for which an upcall has failed (lost packets) must be deleted. > We also need to count them as MISS and LOST. > > Signed-off-by: Daniele Di Proietto > --- > lib/dpif-netdev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index f01fecb..6b61db4 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -2941,6 +2941,8 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > &ufid, DPIF_UC_MISS, NULL, &actions, > &put_actions); > if (OVS_UNLIKELY(error && error != ENOSPC)) { > +dp_packet_delete(packets[i]); > +dp_netdev_count_packet(pmd, DP_STAT_LOST, 1); > continue; > } > > @@ -2984,6 +2986,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > } > } > > +dp_netdev_count_packet(pmd, DP_STAT_MISS, dropped_cnt); > dp_netdev_count_packet(pmd, DP_STAT_LOST, dropped_cnt); > } > > -- > 2.1.4 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCHv4 2/9] dpif-netdev: Group statistics updates in the slow path.
> Since statistics updates might require locking (in future commits) > grouping them will reduce the locking overhead. Do they actually require locking in future commits or is this speculative? Adding locks for these stats seems like a pretty significant step backwards, have you measured the overhead? Ethan > > Signed-off-by: Daniele Di Proietto > --- > lib/dpif-netdev.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 6b61db4..2637e8d 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -2692,10 +2692,6 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, > struct dp_packet *packet_, > { > struct dp_netdev *dp = pmd->dp; > > -if (type == DPIF_UC_MISS) { > -dp_netdev_count_packet(pmd, DP_STAT_MISS, 1); > -} > - > if (OVS_UNLIKELY(!dp->upcall_cb)) { > return ENODEV; > } > @@ -2907,6 +2903,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > if (OVS_UNLIKELY(any_miss) && !fat_rwlock_tryrdlock(&dp->upcall_rwlock)) > { > uint64_t actions_stub[512 / 8], slow_stub[512 / 8]; > struct ofpbuf actions, put_actions; > +int miss_cnt = 0, lost_cnt = 0; > ovs_u128 ufid; > > ofpbuf_use_stub(&actions, actions_stub, sizeof actions_stub); > @@ -2931,6 +2928,8 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > continue; > } > > +miss_cnt++; > + > miniflow_expand(&keys[i].mf, &match.flow); > > ofpbuf_clear(&actions); > @@ -2942,7 +2941,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > &put_actions); > if (OVS_UNLIKELY(error && error != ENOSPC)) { > dp_packet_delete(packets[i]); > -dp_netdev_count_packet(pmd, DP_STAT_LOST, 1); > +lost_cnt++; > continue; > } > > @@ -2976,6 +2975,8 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd, > ofpbuf_uninit(&actions); > ofpbuf_uninit(&put_actions); > fat_rwlock_unlock(&dp->upcall_rwlock); > +dp_netdev_count_packet(pmd, DP_STAT_MISS, miss_cnt); > +dp_netdev_count_packet(pmd, DP_STAT_LOST, lost_cnt); > } else if (OVS_UNLIKELY(any_miss)) { > int dropped_cnt = 0; > > -- > 2.1.4 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCHv4 5/9] dpif-netdev: Use u64_stats_sync when reading/writing stats.
You're not going to like my review of this patch =) So the code is very high quality, though I haven't read it super closely yet. My main question, is if this is really the right approach to handle stats in the DPDK fast path in general. So here's my question: why not just use atomic variables for the members of dp_netdev_flow_stats and dp_netdev_pmd_stats? If you use relaxed reads and writes (no increments), then there's zero performance penalty on x86. Also it has the added side benefits of not requiring additional error-prone multiplatform compat code, saves us having to expose the seqlock, and makes it impossible to access the variables without locking properly. All said, we get the exact same performance profile of this proposed series, with significantly simpler and less error-prone code. What do you think? Ethan On Fri, Mar 27, 2015 at 9:29 AM, Daniele Di Proietto wrote: > While the values are written only by a single thread, reading them > concurrently might produce incorrect results, given the lack of 64-bit > atomic loads on 32-bit systems. > > This fix prevent unconsistent reads of 64-bit values on 32-bit systems. > > Signed-off-by: Daniele Di Proietto > --- > lib/dpif-netdev.c | 48 +--- > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 2637e8d..ba677eb 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -64,6 +64,7 @@ > #include "sset.h" > #include "timeval.h" > #include "tnl-arp-cache.h" > +#include "u64-stats-sync.h" > #include "unixctl.h" > #include "util.h" > #include "openvswitch/vlog.h" > @@ -301,6 +302,9 @@ struct dp_netdev_flow { > > /* Statistics. */ > struct dp_netdev_flow_stats stats; > +/* Used to protect 'stats'. Only guarantees consistency of single stats > + * members, not of the structure as a whole */ > +struct u64_stats_sync stats_lock; > > /* Actions. */ > OVSRCU_TYPE(struct dp_netdev_actions *) actions; > @@ -382,6 +386,9 @@ struct dp_netdev_pmd_thread { > > /* Statistics. */ > struct dp_netdev_pmd_stats stats; > +/* Used to protect 'stats'. Only guarantees consistency of single stats > + * members, not of the structure as a whole */ > +struct u64_stats_sync stats_lock; > > struct latch exit_latch;/* For terminating the pmd thread. */ > atomic_uint change_seq; /* For reloading pmd ports. */ > @@ -754,10 +761,19 @@ dpif_netdev_get_stats(const struct dpif *dpif, struct > dpif_dp_stats *stats) > > stats->n_flows = stats->n_hit = stats->n_missed = stats->n_lost = 0; > CMAP_FOR_EACH (pmd, node, &dp->poll_threads) { > +struct dp_netdev_pmd_stats s; > +uint32_t c; > + > +do { > +c = u64_stats_read_begin(&pmd->stats_lock); > +s = pmd->stats; > +} while (!u64_stats_read_correct(&pmd->stats_lock, c)); > + > stats->n_flows += cmap_count(&pmd->flow_table); > -stats->n_hit += pmd->stats.n[DP_STAT_HIT]; > -stats->n_missed += pmd->stats.n[DP_STAT_MISS]; > -stats->n_lost += pmd->stats.n[DP_STAT_LOST]; > + > +stats->n_hit += s.n[DP_STAT_HIT]; > +stats->n_missed += s.n[DP_STAT_MISS]; > +stats->n_lost += s.n[DP_STAT_LOST]; > } > stats->n_masks = UINT32_MAX; > stats->n_mask_hit = UINT64_MAX; > @@ -1548,10 +1564,15 @@ static void > get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow, > struct dpif_flow_stats *stats) > { > -stats->n_packets = netdev_flow->stats.packet_count; > -stats->n_bytes = netdev_flow->stats.byte_count; > -stats->used = netdev_flow->stats.used; > -stats->tcp_flags = netdev_flow->stats.tcp_flags; > +uint32_t c; > +do { > +c = u64_stats_read_begin(&netdev_flow->stats_lock); > + > +stats->n_packets = netdev_flow->stats.packet_count; > +stats->n_bytes = netdev_flow->stats.byte_count; > +stats->used = netdev_flow->stats.used; > +stats->tcp_flags = netdev_flow->stats.tcp_flags; > +} while (!u64_stats_read_correct(&netdev_flow->stats_lock, c)); > } > > /* Converts to the dpif_flow format, using 'key_buf' and 'mask_buf' for > @@ -1735,6 +1756,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd, > /* Do not allocate extra space. */ > flow = xmalloc(sizeof *flow - sizeof flow->cr.flow.mf + mask.len); > memset(&flow->stats, 0, sizeof flow->stats); > +memset(&flow->stats_lock, 0, sizeof flow->stats_lock); > flow->dead = false; > *CONST_CAST(int *, &flow->pmd_id) = pmd->core_id; > *CONST_CAST(struct flow *, &flow->flow) = match->flow; > @@ -2670,18 +2692,30 @@ dp_netdev_flow_used(struct dp_netdev_flow > *netdev_flow, int cnt, int size, > uint16_t tcp_flags) > { > long long int now = time_msec(); > +uint32_t c; > + > +c = u64_stats_write_begin(&netdev_flow->stats
[ovs-dev] [PATCH 4/9] tunneling: Use flow flag for GRE checksum calculation.
The indication to calculate the GRE checksum is currently the port config rather than the tunnel flow. Currently there is a one to one mapping between the two so there is no difference. However, the kernel datapath must use the flow and it is also potentially more flexible, so this switches how we decide whether to calculate the checksum. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index a9639d3..0e0d791 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -1040,7 +1040,7 @@ netdev_gre_build_header(const struct netdev *netdev, greh->flags = 0; options = (ovs_16aligned_be32 *) (greh + 1); -if (tnl_cfg->csum) { +if (tnl_flow->tunnel.flags & FLOW_TNL_F_CSUM) { greh->flags |= htons(GRE_CSUM); put_16aligned_be32(options, 0); options++; -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/9] tunneling: Include IP TTL in flow metadata.
The IP TTL is currently omitted in the extracted tunnel information that is stored in the flow for userspace tunneling. This includes it so that the same logic used by the kernel also applies. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 211edc7..d4de0d1 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -842,6 +842,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl) tnl->ip_src = get_16aligned_be32(&nh->ip_src); tnl->ip_dst = get_16aligned_be32(&nh->ip_dst); tnl->ip_tos = nh->ip_tos; +tnl->ip_ttl = nh->ip_ttl; return l4; } -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/9] tunneling: Fix location of GRE checksums.
The GRE checksum is a 16 bit field stored in a 32 bit option (the rest is reserved). The current code treats the checksum as a 32-bit field and places it in the right place for little endian systems but not big endian. This fixes the problem by storing the 16 bit field directly. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 6 ++ lib/odp-util.c | 8 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 1ee68bc..a9639d3 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -996,10 +996,8 @@ netdev_gre_push_header__(struct dp_packet *packet, greh = push_ip_header(packet, header, size, &ip_tot_size); if (greh->flags & htons(GRE_CSUM)) { -ovs_16aligned_be32 *options = (ovs_16aligned_be32 *) (greh + 1); - -put_16aligned_be32(options, - (OVS_FORCE ovs_be32) csum(greh, ip_tot_size - sizeof (struct ip_header))); +ovs_be16 *csum_opt = (ovs_be16 *) (greh + 1); +*csum_opt = csum(greh, ip_tot_size - sizeof (struct ip_header)); } } diff --git a/lib/odp-util.c b/lib/odp-util.c index 827b91c..5990edc 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -565,7 +565,7 @@ format_odp_tnl_push_header(struct ds *ds, struct ovs_action_push_tnl *data) greh->flags, ntohs(greh->protocol)); options = (ovs_16aligned_be32 *)(greh + 1); if (greh->flags & htons(GRE_CSUM)) { -ds_put_format(ds, ",csum=0x%"PRIx32, ntohl(get_16aligned_be32(options))); +ds_put_format(ds, ",csum=0x%"PRIx16, ntohs(*((ovs_be16 *)options))); options++; } if (greh->flags & htons(GRE_KEY)) { @@ -913,12 +913,12 @@ ovs_parse_tnl_push(const char *s, struct ovs_action_push_tnl *data) ovs_16aligned_be32 *options = (ovs_16aligned_be32 *) (greh + 1); if (greh->flags & htons(GRE_CSUM)) { -uint32_t csum; +uint16_t csum; -if (!ovs_scan_len(s, &n, ",csum=0x%"SCNx32, &csum)) { +if (!ovs_scan_len(s, &n, ",csum=0x%"SCNx16, &csum)) { return -EINVAL; } -put_16aligned_be32(options, htonl(csum)); +*((ovs_be16 *)options) = htons(csum); options++; } if (greh->flags & htons(GRE_KEY)) { -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 8/9] packet: Add IP pseudoheader checksum calculation.
As OVS adds userspace support for being the endpoint in protocols like tunnels, it will need to be able to calculate pseudoheaders as part of the checksum calculation. Signed-off-by: Jesse Gross --- lib/packets.c | 14 ++ lib/packets.h | 1 + tests/library.at | 2 +- tests/test-csum.c | 31 +++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/packets.c b/lib/packets.c index 07cf2eb..419c6af 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -1043,3 +1043,17 @@ compose_arp(struct dp_packet *b, const uint8_t eth_src[ETH_ADDR_LEN], dp_packet_set_frame(b, eth); dp_packet_set_l3(b, arp); } + +uint32_t +packet_csum_pseudoheader(const struct ip_header *ip) +{ +uint32_t partial = 0; + +partial = csum_add32(partial, get_16aligned_be32(&ip->ip_src)); +partial = csum_add32(partial, get_16aligned_be32(&ip->ip_dst)); +partial = csum_add16(partial, htons(ip->ip_proto)); +partial = csum_add16(partial, htons(ntohs(ip->ip_tot_len) - +IP_IHL(ip->ip_ihl_ver) * 4)); + +return partial; +} diff --git a/lib/packets.h b/lib/packets.h index 2bbe6d9..29ea54f 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -806,5 +806,6 @@ void packet_format_tcp_flags(struct ds *, uint16_t); const char *packet_tcp_flag_to_string(uint32_t flag); void compose_arp(struct dp_packet *b, const uint8_t eth_src[ETH_ADDR_LEN], ovs_be32 ip_src, ovs_be32 ip_dst); +uint32_t packet_csum_pseudoheader(const struct ip_header *); #endif /* packets.h */ diff --git a/tests/library.at b/tests/library.at index 2507688..426b206 100644 --- a/tests/library.at +++ b/tests/library.at @@ -7,7 +7,7 @@ AT_CHECK([ovstest test-flows flows pcap], [0], [checked 247 packets, 0 errors AT_CLEANUP AT_SETUP([test TCP/IP checksumming]) -AT_CHECK([ovstest test-csum], [0], [####### +AT_CHECK([ovstest test-csum], [0], [#### ]) AT_CLEANUP diff --git a/tests/test-csum.c b/tests/test-csum.c index e25fb3d..2685f67 100644 --- a/tests/test-csum.c +++ b/tests/test-csum.c @@ -20,11 +20,13 @@ #include #include #include +#include #include #include #include #include "crc32c.h" #include "ovstest.h" +#include "packets.h" #include "random.h" #include "unaligned.h" #include "util.h" @@ -175,6 +177,34 @@ test_crc32c(void) mark('#'); } +/* Check the IP pseudoheader calculation. */ +static void +test_pseudo(void) +{ +/* Try an IP header similar to one that the tunnel code + * might generate. */ +struct ip_header ip = { +.ip_ihl_ver = IP_IHL_VER(5, 4), +.ip_tos = 0, +.ip_tot_len = htons(134), +.ip_id = 0, +.ip_frag_off = htons(IP_DF), +.ip_ttl = 64, +.ip_proto = IPPROTO_UDP, +.ip_csum = htons(0x1265), +.ip_src = { .hi = htons(0x1400), .lo = htons(0x0002) }, +.ip_dst = { .hi = htons(0x1400), .lo = htons(0x0001) } +}; + +assert(packet_csum_pseudoheader(&ip) == 0x8628); + +/* And also test something totally different to check for + * corner cases. */ +memset(&ip, 0xff, sizeof ip); +assert(packet_csum_pseudoheader(&ip) == 0x5c2fb); + +mark('#'); +} static void test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) @@ -239,6 +269,7 @@ test_csum_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) test_rfc1624(); test_crc32c(); +test_pseudo(); /* Test recalc_csum16(). */ for (i = 0; i < 32; i++) { -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 6/9] tunneling: Factor out common UDP tunnel code.
Currently, the userspace VXLAN implementation contains the code for generating and parsing both the UDP and VXLAN headers. This pulls out the UDP portion for better layering and to make it easier to support additional UDP based tunnels and features. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 105 - lib/odp-util.c | 37 +++ 2 files changed, 85 insertions(+), 57 deletions(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 0e0d791..0c9f5a4 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -876,6 +876,65 @@ push_ip_header(struct dp_packet *packet, return ip + 1; } +static void * +udp_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl) +{ +struct udp_header *udp; + +udp = ip_extract_tnl_md(packet, tnl); +if (!udp) { +return NULL; +} + +tnl->tp_src = udp->udp_src; +tnl->tp_dst = udp->udp_dst; + +return udp + 1; +} + +static ovs_be16 +get_src_port(struct dp_packet *packet) +{ +uint32_t hash; + +hash = dp_packet_get_dp_hash(packet); + +return htonsuint64_t) hash * (tnl_udp_port_max - tnl_udp_port_min)) >> 32) + + tnl_udp_port_min); +} + +static void * +push_udp_header(struct dp_packet *packet, const void *header, int size) +{ +struct udp_header *udp; +int ip_tot_size; + +udp = push_ip_header(packet, header, size, &ip_tot_size); + +/* set udp src port */ +udp->udp_src = get_src_port(packet); +udp->udp_len = htons(ip_tot_size - sizeof (struct ip_header)); +/* udp_csum is zero */ + +return udp + 1; +} + +static void * +udp_build_header(struct netdev_tunnel_config *tnl_cfg, + struct ovs_action_push_tnl *data) +{ +struct ip_header *ip; +struct udp_header *udp; + +ip = ip_hdr(data->header); +ip->ip_proto = IPPROTO_UDP; + +udp = (struct udp_header *) (ip + 1); +udp->udp_dst = tnl_cfg->dst_port; + +return udp + 1; +} + static int gre_header_len(ovs_be16 flags) { @@ -1068,7 +1127,6 @@ vxlan_extract_md(struct dp_packet *packet) { struct pkt_metadata *md = &packet->md; struct flow_tnl *tnl = &md->tunnel; -struct udp_header *udp; struct vxlanhdr *vxh; memset(md, 0, sizeof *md); @@ -1076,11 +1134,10 @@ vxlan_extract_md(struct dp_packet *packet) return; } -udp = ip_extract_tnl_md(packet, tnl); -if (!udp) { +vxh = udp_extract_tnl_md(packet, tnl); +if (!vxh) { return; } -vxh = (struct vxlanhdr *) (udp + 1); if (get_16aligned_be32(&vxh->vx_flags) != htonl(VXLAN_FLAGS) || (get_16aligned_be32(&vxh->vx_vni) & htonl(0xff))) { @@ -1090,8 +1147,6 @@ vxlan_extract_md(struct dp_packet *packet) reset_tnl_md(md); return; } -tnl->tp_src = udp->udp_src; -tnl->tp_dst = udp->udp_dst; tnl->tun_id = htonll(ntohl(get_16aligned_be32(&vxh->vx_vni)) >> 8); dp_packet_reset_packet(packet, VXLAN_HLEN); @@ -1116,21 +1171,14 @@ netdev_vxlan_build_header(const struct netdev *netdev, { struct netdev_vport *dev = netdev_vport_cast(netdev); struct netdev_tunnel_config *tnl_cfg; -struct ip_header *ip; -struct udp_header *udp; struct vxlanhdr *vxh; /* XXX: RCUfy tnl_cfg. */ ovs_mutex_lock(&dev->mutex); tnl_cfg = &dev->tnl_cfg; -ip = ip_hdr(data->header); -ip->ip_proto = IPPROTO_UDP; +vxh = udp_build_header(tnl_cfg, data); -udp = (struct udp_header *) (ip + 1); -udp->udp_dst = tnl_cfg->dst_port; - -vxh = (struct vxlanhdr *) (udp + 1); put_16aligned_be32(&vxh->vx_flags, htonl(VXLAN_FLAGS)); put_16aligned_be32(&vxh->vx_vni, htonl(ntohll(tnl_flow->tunnel.tun_id) << 8)); @@ -1140,32 +1188,6 @@ netdev_vxlan_build_header(const struct netdev *netdev, return 0; } -static ovs_be16 -get_src_port(struct dp_packet *packet) -{ -uint32_t hash; - -hash = dp_packet_get_dp_hash(packet); - -return htonsuint64_t) hash * (tnl_udp_port_max - tnl_udp_port_min)) >> 32) + - tnl_udp_port_min); -} - -static void -netdev_vxlan_push_header__(struct dp_packet *packet, - const void *header, int size) -{ -struct udp_header *udp; -int ip_tot_size; - -udp = push_ip_header(packet, header, size, &ip_tot_size); - -/* set udp src port */ -udp->udp_src = get_src_port(packet); -udp->udp_len = htons(ip_tot_size - sizeof (struct ip_header)); -/* udp_csum is zero */ -} - static int netdev_vxlan_push_header(const struct netdev *netdev OVS_UNUSED, struct dp_packet **packets, int cnt, @@ -1174,8 +1196,7 @@ netdev_vxlan_push_header(const struct netdev *netdev OVS_UNUSED, int i; for (i = 0; i < cnt; i++) { -netdev_vxlan_push_header__(packets[i], - data->header, VXLAN_HLEN); +push_udp_header(packets[i], data->header, VXLAN_HLEN); packet
[ovs-dev] [PATCH 7/9] tunneling: Add userspace tunnel support for Geneve.
This adds basic userspace dataplane support for the Geneve tunneling protocol. The rest of userspace only has the ability to handle Geneve without options and this follows that pattern for the time being. However, when the rest of userspace is updated it should be easy to extend the dataplane as well. Signed-off-by: Jesse Gross --- NEWS | 4 +- lib/netdev-vport.c | 114 ++- lib/odp-util.c | 28 ++-- lib/packets.h| 19 tests/odp.at | 1 + tests/tunnel-push-pop.at | 16 +++ 6 files changed, 176 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 9f9dc4c..87460a7 100644 --- a/NEWS +++ b/NEWS @@ -62,8 +62,8 @@ Post-v2.3.0 - A simple wrapper script, 'ovs-docker', to integrate OVS with Docker containers. If and when there is a native integration of Open vSwitch with Docker, the wrapper script will be retired. - - Added support for DPDK Tunneling. VXLAN and GRE are supported protocols. - This is generic tunneling mechanism for userspace datapath. + - Added support for DPDK Tunneling. VXLAN, GRE, and Geneve are supported + protocols. This is generic tunneling mechanism for userspace datapath. - Support for multicast snooping (IGMPv1 and IGMPv2) - Support for Linux kernels up to 3.19.x - The documentation now use the term 'destination' to mean one of syslog, diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 0c9f5a4..ef96862 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -61,6 +61,11 @@ static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5); sizeof(struct udp_header) + \ sizeof(struct vxlanhdr)) +#define GENEVE_BASE_HLEN (sizeof(struct eth_header) + \ +sizeof(struct ip_header) + \ +sizeof(struct udp_header) + \ +sizeof(struct genevehdr)) + #define DEFAULT_TTL 64 struct netdev_vport { @@ -1203,6 +1208,111 @@ netdev_vxlan_push_header(const struct netdev *netdev OVS_UNUSED, } static void +geneve_extract_md(struct dp_packet *packet) +{ +struct pkt_metadata *md = &packet->md; +struct flow_tnl *tnl = &md->tunnel; +struct genevehdr *gnh; +unsigned int hlen; + +memset(md, 0, sizeof *md); +if (GENEVE_BASE_HLEN > dp_packet_size(packet)) { +VLOG_WARN_RL(&err_rl, "geneve packet too small: min header=%u packet size=%u\n", + (unsigned int)GENEVE_BASE_HLEN, dp_packet_size(packet)); +return; +} + +gnh = udp_extract_tnl_md(packet, tnl); +if (!gnh) { +return; +} + +hlen = GENEVE_BASE_HLEN + gnh->opt_len * 4; +if (hlen > dp_packet_size(packet)) { +VLOG_WARN_RL(&err_rl, "geneve packet too small: header len=%u packet size=%u\n", + hlen, dp_packet_size(packet)); +reset_tnl_md(md); +return; +} + +if (gnh->ver != 0) { +VLOG_WARN_RL(&err_rl, "unknown geneve version: %"PRIu8"\n", gnh->ver); +reset_tnl_md(md); +return; +} + +if (gnh->opt_len && gnh->critical) { +VLOG_WARN_RL(&err_rl, "unknown geneve critical options: %"PRIu8" bytes\n", + gnh->opt_len * 4); +reset_tnl_md(md); +return; +} + +if (gnh->proto_type != htons(ETH_TYPE_TEB)) { +VLOG_WARN_RL(&err_rl, "unknown geneve encapsulated protocol: %#x\n", + ntohs(gnh->proto_type)); +reset_tnl_md(md); +return; +} + +tnl->flags |= gnh->oam ? FLOW_TNL_F_OAM : 0; +tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gnh->vni)) >> 8); + +dp_packet_reset_packet(packet, hlen); +} + +static int +netdev_geneve_pop_header(struct netdev *netdev_ OVS_UNUSED, + struct dp_packet **pkt, int cnt) +{ +int i; + +for (i = 0; i < cnt; i++) { +geneve_extract_md(pkt[i]); +} +return 0; +} + +static int +netdev_geneve_build_header(const struct netdev *netdev, + struct ovs_action_push_tnl *data, + const struct flow *tnl_flow) +{ +struct netdev_vport *dev = netdev_vport_cast(netdev); +struct netdev_tunnel_config *tnl_cfg; +struct genevehdr *gnh; + +/* XXX: RCUfy tnl_cfg. */ +ovs_mutex_lock(&dev->mutex); +tnl_cfg = &dev->tnl_cfg; + +gnh = udp_build_header(tnl_cfg, data); + +gnh->oam = !!(tnl_flow->tunnel.flags & FLOW_TNL_F_OAM); +gnh->proto_type = htons(ETH_TYPE_TEB); +put_16aligned_be32(&gnh->vni, htonl(ntohll(tnl_flow->tunnel.tun_id) << 8)); + +ovs_mutex_unlock(&dev->mutex); +data->header_len = GENEVE_BASE_HLEN; +data->tnl_type = OVS_VPORT_TYPE_GENEVE; +return 0; +} + +static int +netdev_geneve_push_header(const struct netdev *netdev OVS_UNUSED, + struct dp_packet **packets
[ovs-dev] [PATCH 9/9] tunneling: Add UDP checksum support for userspace tunnels.
Kernel based OVS recently added the ability to support checksums for UDP based tunnels (Geneve and VXLAN). This adds similar support for the userspace datapath to bring feature parity. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 37 ++--- lib/odp-util.c | 15 +++ tests/odp.at | 5 +++-- tests/tunnel-push-pop.at | 10 +- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index ef96862..d39d449 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -891,6 +891,18 @@ udp_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl) return NULL; } +if (udp->udp_csum) { +uint32_t csum = packet_csum_pseudoheader(dp_packet_l3(packet)); + +csum = csum_continue(csum, udp, dp_packet_size(packet) - + ((const unsigned char *)udp - + (const unsigned char *)dp_packet_l2(packet))); +if (csum_finish(csum)) { +return NULL; +} +tnl->flags |= FLOW_TNL_F_CSUM; +} + tnl->tp_src = udp->udp_src; tnl->tp_dst = udp->udp_dst; @@ -919,13 +931,25 @@ push_udp_header(struct dp_packet *packet, const void *header, int size) /* set udp src port */ udp->udp_src = get_src_port(packet); udp->udp_len = htons(ip_tot_size - sizeof (struct ip_header)); -/* udp_csum is zero */ + +if (udp->udp_csum) { +uint32_t csum = packet_csum_pseudoheader(ip_hdr(dp_packet_data(packet))); + +csum = csum_continue(csum, udp, + ip_tot_size - sizeof (struct ip_header)); +udp->udp_csum = csum_finish(csum); + +if (!udp->udp_csum) { +udp->udp_csum = htons(0x); +} +} return udp + 1; } static void * udp_build_header(struct netdev_tunnel_config *tnl_cfg, + const struct flow *tnl_flow, struct ovs_action_push_tnl *data) { struct ip_header *ip; @@ -937,6 +961,13 @@ udp_build_header(struct netdev_tunnel_config *tnl_cfg, udp = (struct udp_header *) (ip + 1); udp->udp_dst = tnl_cfg->dst_port; +if (tnl_flow->tunnel.flags & FLOW_TNL_F_CSUM) { +/* Write a value in now to mark that we should compute the checksum + * later. 0x is handy because it is transparent to the + * calculation. */ +udp->udp_csum = htons(0x); +} + return udp + 1; } @@ -1182,7 +1213,7 @@ netdev_vxlan_build_header(const struct netdev *netdev, ovs_mutex_lock(&dev->mutex); tnl_cfg = &dev->tnl_cfg; -vxh = udp_build_header(tnl_cfg, data); +vxh = udp_build_header(tnl_cfg, tnl_flow, data); put_16aligned_be32(&vxh->vx_flags, htonl(VXLAN_FLAGS)); put_16aligned_be32(&vxh->vx_vni, htonl(ntohll(tnl_flow->tunnel.tun_id) << 8)); @@ -1286,7 +1317,7 @@ netdev_geneve_build_header(const struct netdev *netdev, ovs_mutex_lock(&dev->mutex); tnl_cfg = &dev->tnl_cfg; -gnh = udp_build_header(tnl_cfg, data); +gnh = udp_build_header(tnl_cfg, tnl_flow, data); gnh->oam = !!(tnl_flow->tunnel.flags & FLOW_TNL_F_OAM); gnh->proto_type = htons(ETH_TYPE_TEB); diff --git a/lib/odp-util.c b/lib/odp-util.c index 47dfc92..3e6a7bd 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -516,8 +516,9 @@ format_udp_tnl_push_header(struct ds *ds, const struct ip_header *ip) const struct udp_header *udp; udp = (const struct udp_header *) (ip + 1); -ds_put_format(ds, "udp(src=%"PRIu16",dst=%"PRIu16"),", - ntohs(udp->udp_src), ntohs(udp->udp_dst)); +ds_put_format(ds, "udp(src=%"PRIu16",dst=%"PRIu16",csum=0x%"PRIx16"),", + ntohs(udp->udp_src), ntohs(udp->udp_dst), + ntohs(udp->udp_csum)); return udp + 1; } @@ -854,7 +855,7 @@ ovs_parse_tnl_push(const char *s, struct ovs_action_push_tnl *data) struct ip_header *ip; struct udp_header *udp; struct gre_base_hdr *greh; -uint16_t gre_proto, dl_type, udp_src, udp_dst; +uint16_t gre_proto, dl_type, udp_src, udp_dst, csum; ovs_be32 sip, dip; uint32_t tnl_type = 0, header_len = 0; void *l3, *l4; @@ -899,14 +900,14 @@ ovs_parse_tnl_push(const char *s, struct ovs_action_push_tnl *data) /* Tunnel header */ udp = (struct udp_header *) l4; greh = (struct gre_base_hdr *) l4; -if (ovs_scan_len(s, &n, "udp(src=%"SCNi16",dst=%"SCNi16"),", - &udp_src, &udp_dst)) { +if (ovs_scan_len(s, &n, "udp(src=%"SCNi16",dst=%"SCNi16",csum=0x%"SCNx16"),", + &udp_src, &udp_dst, &csum)) { uint32_t vx_flags, vni; udp->udp_src = htons(udp_src); udp->udp_dst = htons(udp_dst); udp->udp_len = 0; -udp->udp_csum = 0; +udp->udp_csum = htons(csum); if (ovs_scan_len(s, &n, "vxlan(flags=0x%"SCNx32",vni=0x%"SCNx32"))",
[ovs-dev] [PATCH 2/9] tunneling: Add check for GRE protocol is Ethernet.
On receive, the userspace GRE code doesn't check the protocol field. Since OVS only understands Ethernet packets, this adds a check that the inner protocol is Ethernet and discards other types of packets. Signed-off-by: Jesse Gross --- lib/netdev-vport.c | 4 1 file changed, 4 insertions(+) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index d4de0d1..1ee68bc 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -911,6 +911,10 @@ parse_gre_header(struct dp_packet *packet, return -EINVAL; } +if (greh->protocol != htons(ETH_TYPE_TEB)) { +return -EINVAL; +} + hlen = gre_header_len(greh->flags); if (hlen > dp_packet_size(packet)) { return -EINVAL; -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 5/9] odp-util: Shift VXLAN VNI when printing/parsing.
Currently when printing a userspace tunnel action for VXLAN, the VNI is treated as a 32 bit field rather than 24 bit. Even if this is the representation that we use internally, we should still show the right VNI to avoid confusing people. Signed-off-by: Jesse Gross --- lib/odp-util.c | 4 ++-- tests/odp.at | 2 +- tests/tunnel-push-pop.at | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 5990edc..7c24512 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -552,7 +552,7 @@ format_odp_tnl_push_header(struct ds *ds, struct ovs_action_push_tnl *data) vxh = (const struct vxlanhdr *) (udp + 1); ds_put_format(ds, "vxlan(flags=0x%"PRIx32",vni=0x%"PRIx32")", ntohl(get_16aligned_be32(&vxh->vx_flags)), - ntohl(get_16aligned_be32(&vxh->vx_vni))); + ntohl(get_16aligned_be32(&vxh->vx_vni)) >> 8); } else if (data->tnl_type == OVS_VPORT_TYPE_GRE) { const struct gre_base_hdr *greh; ovs_16aligned_be32 *options; @@ -901,7 +901,7 @@ ovs_parse_tnl_push(const char *s, struct ovs_action_push_tnl *data) return -EINVAL; } put_16aligned_be32(&vxh->vx_flags, htonl(vx_flags)); -put_16aligned_be32(&vxh->vx_vni, htonl(vx_vni)); +put_16aligned_be32(&vxh->vx_vni, htonl(vx_vni << 8)); tnl_type = OVS_VPORT_TYPE_VXLAN; header_len = sizeof *eth + sizeof *ip + sizeof *udp + sizeof *vxh; diff --git a/tests/odp.at b/tests/odp.at index de08771..bc68d35 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -281,7 +281,7 @@ set(tunnel(tun_id=0xabcdef1234567890,src=1.1.1.1,dst=2.2.2.2,tos=0,ttl=64,tp_src tnl_pop(4) tnl_push(tnl_port(4),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x40),gre((flags=0x20,proto=0x6558),key=0x1e241)),out_port(1)) tnl_push(tnl_port(4),header(size=46,type=3,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x40),gre((flags=0xa0,proto=0x6558),csum=0x0,key=0x1e241)),out_port(1)) -tnl_push(tnl_port(6),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x1c700)),out_port(1)) +tnl_push(tnl_port(6),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x1c7)),out_port(1)) ]) AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [`cat actions.txt` diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 6e1c0c1..328a040 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -58,14 +58,14 @@ dnl Check VXLAN tunnel push AT_CHECK([ovs-ofctl add-flow int-br action=2]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], - [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x7b00)),out_port(100)) + [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x7b)),out_port(100)) ]) dnl Check VXLAN tunnel push set tunnel id by flow AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], - [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x7c00)),out_port(100)) + [Datapath actions: tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.93,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=4789),vxlan(flags=0x800,vni=0x7c)),out_port(100)) ]) dnl Check GRE tunnel push -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/9] tunneling: Include IP TTL in flow metadata.
Acked-by: Pritesh Kothari > On Mar 30, 2015, at 3:14 PM, Jesse Gross wrote: > > The IP TTL is currently omitted in the extracted tunnel information > that is stored in the flow for userspace tunneling. This includes it > so that the same logic used by the kernel also applies. > > Signed-off-by: Jesse Gross > --- > lib/netdev-vport.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 211edc7..d4de0d1 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -842,6 +842,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct > flow_tnl *tnl) > tnl->ip_src = get_16aligned_be32(&nh->ip_src); > tnl->ip_dst = get_16aligned_be32(&nh->ip_dst); > tnl->ip_tos = nh->ip_tos; > +tnl->ip_ttl = nh->ip_ttl; > > return l4; > } > -- > 1.9.1 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/9] tunneling: Add check for GRE protocol is Ethernet.
should a test case be added to catch this ? Regards, Pritesh > On Mar 30, 2015, at 3:14 PM, Jesse Gross wrote: > > +if (greh->protocol != htons(ETH_TYPE_TEB)) { > +return -EINVAL; > +} > + ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC HSA 0/4] HSA for OVS prototype.
This series implements a prototype of using Header Space Analysis (HSA) for OVS OpenFlow table analysis. The implementation allows users to find all possible output ports with the input header format reachable from a specified input port. It also allows users to check if there is any loop formed in the OpenFlow table. There are two major limitations of the prototype. Firstly, only a limited set of OFPACT_* actions are supported and extending to work with more actions can be very complicated. Secondly, the implementation is in single thread, and can take 20+ mins to run on a production setup with ~100 OpenFlow rules. Since this is a very complicated work, I'd like to post the prototype out for high-level discussions (e.g. feasibility, design, multi-threading). Sincerely welcome any comments. == Example - Find All Outputs == root:# ovs-appctl hsa/detect-leak br0 100 Header-Space init done: reg0=0,reg1=0,reg2=0,reg3=0,reg4=0,reg5=0,reg6=0,reg7=0,metadata=0,in_port=100 Flows dump from bridge (br0): table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=output:200 table_id=0, priority=2,ip,in_port=100,nw_src=20.2.0.0/16,actions=drop table_id=0, priority=1,tcp,tun_id=0x,in_port=100,nw_src=30.1.0.0/16,tp_src=10,actions=output:200 OUTPUT == Output Port No Input Header Space == == 200 ip,reg0=0,reg1=0,reg2=0,reg3=0,reg4=0,reg5=0,reg6=0,reg7=0,metadata=0,in_port=100,nw_src=10.1.0.0/16 Output Port No Input Header Space == == 200 tcp,reg0=0,reg1=0,reg2=0,reg3=0,reg4=0,reg5=0,reg6=0,reg7=0,tun_id=0x,metadata=0,in_port=100,nw_src=30.1.0.0/16,tp_src=10 Example - Loop Detection root:# ovs-appctl hsa/detect-loop br0 100 Header-Space init done: reg0=0,reg1=0,reg2=0,reg3=0,reg4=0,reg5=0,reg6=0,reg7=0,metadata=0,in_port=100 Flows dump from bridge (br0): table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) OUTPUT == Input Header Space == ip,reg0=0,reg1=0,reg2=0,reg3=0,reg4=0,reg5=0,reg6=0,reg7=0,metadata=0,in_port=100,nw_src=10.1.0.0/16,nw_dst=20.2.0.0/16 = Loop Path (Infinite Loop) = table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) table_id=1, ip,in_port=100,nw_dst=20.2.0.0/16,actions=resubmit(,0) table_id=0, priority=3,ip,in_port=100,nw_src=10.1.0.0/16,actions=resubmit(,1) Alex Wang (4): ovs_list: Extend list functions. hsa-match: Sparse representation of a byte array derived from "struct match". flow: Add supporting functions for HSA. ofprot-dpif-hsa: Implement HSA prototype. lib/automake.mk|2 + lib/flow.c | 43 ++ lib/flow.h |5 + lib/hsa-
[ovs-dev] [RFC HSA 1/4] ovs_list: Extend list functions.
This commit adds function that allows the appending of one list content to the other. Also, it adds functions which allows list to be sorted. Signed-off-by: Alex Wang --- lib/list.h| 67 + tests/library.at |2 +- tests/test-list.c | 106 + 3 files changed, 174 insertions(+), 1 deletion(-) diff --git a/lib/list.h b/lib/list.h index b40bbef..76f930c 100644 --- a/lib/list.h +++ b/lib/list.h @@ -30,10 +30,12 @@ static inline void list_poison(struct ovs_list *); static inline void list_insert(struct ovs_list *, struct ovs_list *); static inline void list_splice(struct ovs_list *before, struct ovs_list *first, struct ovs_list *last); +static inline void list_join(struct ovs_list *dst, struct ovs_list *src); static inline void list_push_front(struct ovs_list *, struct ovs_list *); static inline void list_push_back(struct ovs_list *, struct ovs_list *); static inline void list_replace(struct ovs_list *, const struct ovs_list *); static inline void list_moved(struct ovs_list *, const struct ovs_list *orig); +static inline void list_swap(struct ovs_list *, struct ovs_list *); static inline void list_move(struct ovs_list *dst, struct ovs_list *src); /* List removal. */ @@ -44,6 +46,8 @@ static inline struct ovs_list *list_pop_back(struct ovs_list *); /* List elements. */ static inline struct ovs_list *list_front(const struct ovs_list *); static inline struct ovs_list *list_back(const struct ovs_list *); +static inline struct ovs_list *list_at_position(const struct ovs_list *, +size_t n); /* List properties. */ static inline size_t list_size(const struct ovs_list *); @@ -101,6 +105,18 @@ list_insert(struct ovs_list *before, struct ovs_list *elem) before->prev = elem; } +/* Appends 'src''s content to the end of 'dst'. After 'list_join', 'src' + * becomes an empty list. */ +static inline void +list_join(struct ovs_list *dst, struct ovs_list *src) +{ +if (list_is_empty(src)) { +return; +} + +list_splice(dst, list_front(src), src); +} + /* Removes elements 'first' though 'last' (exclusive) from their current list, then inserts them just before 'before'. */ static inline void @@ -149,6 +165,38 @@ list_replace(struct ovs_list *element, const struct ovs_list *position) element->prev->next = element; } +/* Swaps the two adjacent list elements. 'ahead' must be ahead of 'after'. */ +static inline void +list_swap_adjacent(struct ovs_list *ahead, struct ovs_list *after) +{ +ahead->next = after->next; +ahead->next->prev = ahead; +after->prev = ahead->prev; +ahead->prev->next = after; +ahead->prev = after; +after->next = ahead; +} + +/* Swaps the position of two list nodes, 'elem1' and 'elem2'. */ +static inline void +list_swap(struct ovs_list *elem1, struct ovs_list *elem2) +{ +if (elem1 == elem2) { +return; +} else if (elem1->prev == elem2) { +/* element2 is ahead of element1. */ +list_swap_adjacent(elem2, elem1); +} else if (elem2->prev == elem1) { +/* element1 is ahead of element2. */ +list_swap_adjacent(elem1, elem2); +} else { +const struct ovs_list tmp = *elem1; + +list_replace(elem1, elem2); +list_replace(elem2, &tmp); +} +} + /* Adjusts pointers around 'list' to compensate for 'list' having been moved * around in memory (e.g. as a consequence of realloc()), with original * location 'orig'. @@ -233,6 +281,25 @@ list_back(const struct ovs_list *list_) return list->prev; } +/* Returns the element at position 'n', assumes the first element is + * at index 0. The 'list_' must contain at least 'n' elements. */ +static inline struct ovs_list * +list_at_position(const struct ovs_list *list_, size_t n) +{ +struct ovs_list *list = CONST_CAST(struct ovs_list *, list_); +struct ovs_list *ret; +size_t cnt = 0; + +for (ret = list->next; ret != list; ret = ret->next) { +if (cnt++ == n) { +return ret; +} +} +ovs_assert(false); + +return NULL; +} + /* Returns the number of elements in 'list'. Runs in O(n) in the number of elements. */ static inline size_t diff --git a/tests/library.at b/tests/library.at index 2507688..fab7402 100644 --- a/tests/library.at +++ b/tests/library.at @@ -38,7 +38,7 @@ AT_CHECK([ovstest test-atomic]) AT_CLEANUP AT_SETUP([test linked lists]) -AT_CHECK([ovstest test-list], [0], [.. +AT_CHECK([ovstest test-list], [0], [.. ]) AT_CLEANUP diff --git a/tests/test-list.c b/tests/test-list.c index 5fd7149..3652064 100644 --- a/tests/test-list.c +++ b/tests/test-list.c @@ -20,6 +20,7 @@ #include #undef NDEBUG #include "list.h" +#include "sort.h" #include #include #include "ovstest.h" @@ -160,6 +161,107 @@ test_list_for_each_safe(void) } static void +test_list_swap_elem(void)
[ovs-dev] [RFC HSA 2/4] hsa-match: Sparse representation of a byte array derived from "struct match".
For conducting Header Space Analysis (HSA), we convert the wildcarded OpenFlow flow represented by 'struct match' into an encoded byte array. To further save memory, we use a sparse array to represent such byte array in the same way as 'struct miniflow'. So, this commit implements the structs and functions for conversion between sparse array and 'struct match'. This commit also implements the set operations (e.g. intersect, union, complementation and difference) as functions. Signed-off-by: Alex Wang --- lib/automake.mk|2 + lib/hsa-match.c| 374 lib/hsa-match.h| 92 tests/automake.mk |2 + tests/library.at |5 + tests/test-hsa-match.c | 171 ++ 6 files changed, 646 insertions(+) create mode 100644 lib/hsa-match.c create mode 100644 lib/hsa-match.h create mode 100644 tests/test-hsa-match.c diff --git a/lib/automake.mk b/lib/automake.mk index 3629079..09b7a64 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -111,6 +111,8 @@ lib_libopenvswitch_la_SOURCES = \ lib/mac-learning.h \ lib/match.c \ lib/match.h \ + lib/hsa-match.c \ + lib/hsa-match.h \ lib/mcast-snooping.c \ lib/mcast-snooping.h \ lib/memory.c \ diff --git a/lib/hsa-match.c b/lib/hsa-match.c new file mode 100644 index 000..3d69996 --- /dev/null +++ b/lib/hsa-match.c @@ -0,0 +1,374 @@ +/* + * Copyright (c) 2015 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "hsa-match.h" + +#include "util.h" + +/* Given 'hsbm' and 'match', initializes 'hsbm' with 'match'. + * Caller must guarantee, 'hsbm->values' is not assigned. */ +void +hsbm_init(struct hsbm *hsbm, const struct match *match) +{ +uint32_t *flow = (uint32_t *) &match->flow; +uint32_t *wc = (uint32_t *) &match->wc; +size_t sz = 0; +size_t i, j; + +hsbm->map = 0; +hsbm->values = NULL; + +/* Encodes every 4 bytes from 'match' to to 8 bytes and sets the + * 'hsbm->map' and 'hsbm->values' correctly. */ +for (i = 0; i < HSBM_VALUES_MAX; i++) { +uint64_t encoded_value = 0; + +for (j = 0; j < 32; j++) { +uint8_t flow_bit = (flow[i] >> j) & 0x1; +uint8_t wc_bit = (wc[i] >> j) & 0x1; + +/* If wc_bit is set, checks the bit. Otherwise, sets to 'x'. */ +if (wc_bit) { +encoded_value |= ((uint64_t) (flow_bit ? HSBM_VALUE_BIT_EM_ONE + : HSBM_VALUE_BIT_EM_ZERO) << (2*j)); +} else { +encoded_value |= ((uint64_t) HSBM_VALUE_BIT_WC << (2*j)); +} +} + +if (encoded_value == UINT64_MAX) { +hsbm->map |= (uint64_t) 0x1 << i; +} else { +hsbm->values = xrealloc(hsbm->values, + (++sz) * sizeof *hsbm->values); +hsbm->values[sz-1] = encoded_value; +} +} +} + +/* Initializes 'hsbm' such that only one bit 'map_idx' in 'hsbm->map' is + * HSBM_MAP_BIT_NOT_WC. Correspondingly, sets the 'values[0]' so that + * the 'bit_idx' bit is set to 'bit_val'. */ +void +hsbm_init_one_bit(struct hsbm *hsbm, size_t map_idx, size_t bit_idx, + size_t bit_val) +{ +ovs_assert(map_idx < HSBM_VALUES_MAX); +hsbm->map = ((uint64_t) 1 << HSBM_VALUES_MAX) - 1; +hsbm->map &= ~((uint64_t) 1 << map_idx); +hsbm->values = xzalloc(sizeof *hsbm->values); +hsbm->values[0] = ~((uint64_t) HSBM_VALUE_BIT_WC << 2*bit_idx) +| ((uint64_t) bit_val << 2*bit_idx); +} + +/* Frees the 'values' pointer if it is non-NULL. */ +void +hsbm_uninit(struct hsbm *hsbm) +{ +if (hsbm->values) { +free(hsbm->values); +} +} + +/* Destroys the 'hsbm'. */ +void +hsbm_destroy(struct hsbm *hsbm) +{ +hsbm_uninit(hsbm); +free(hsbm); +} + +/* Converts the 'hsbm' back to 'match'. */ +void +hsbm_to_match(struct match *match, const struct hsbm *hsbm) +{ +uint32_t *flow = (uint32_t *) &match->flow; +uint32_t *wc = (uint32_t *) &match->wc; +size_t idx = 0; +size_t i; + +for (i = 0; i < HSBM_VALUES_MAX; i++) { +if ((hsbm->map >> i) & 0x1) { +flow[i] = wc[i] = 0; +} else { +uint64_t encoded_value = hsbm->values[idx++]; +uint32_t flow_value = 0; +uint32_t wc_value = 0; +size_t j; +
[ovs-dev] [RFC HSA 3/4] flow: Add supporting functions for HSA.
This commit adds functions that check if the 'struct flow_wildcards' member is fully masked (exact-match) or fully unmasked (don't care the entire field at all). Also, this commit adds a function to shape a less-masked flow field by a more-masked flow field. Signed-off-by: Alex Wang --- lib/flow.c | 43 +++ lib/flow.h |5 + 2 files changed, 48 insertions(+) diff --git a/lib/flow.c b/lib/flow.c index e54280a..0608503 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -898,9 +898,52 @@ flow_print(FILE *stream, const struct flow *flow) fputs(s, stream); free(s); } + +/* Applies the 'src_field' and 'src_wc' to 'dst_field' and 'dst_wc' + * respectively. This should be used to shape a less-masked field by + * a more-masked field. */ +void +flow_apply_field(void *dst_field, void *dst_wc, const void *src_field, + const void *src_wc, size_t size) +{ +const uint8_t *sf = src_field; +const uint8_t *sw = src_wc; +uint8_t *df = dst_field; +uint8_t *dw = dst_wc; +size_t i; + +for (i = 0; i < size ; i++) { +df[i] = sf[i] | df[i]; +dw[i] = sw[i] | dw[i]; +} +} /* flow_wildcards functions. */ +/* Returns true if the 'field' of 'len' byte long is + * all one. */ +bool +flow_wildcard_is_fully_masked(void *field, size_t len) +{ +char cmp[len]; + +memset(cmp, 0xff, len); + +return !memcmp(field, cmp, len); +} + +/* Returns true if the 'field' of 'len' byte long is + * all zero. */ +bool +flow_wildcard_is_fully_unmasked(void *field, size_t len) +{ +char cmp[len]; + +memset(cmp, 0, len); + +return !memcmp(field, cmp, len); +} + /* Initializes 'wc' as a set of wildcards that matches every packet. */ void flow_wildcards_init_catchall(struct flow_wildcards *wc) diff --git a/lib/flow.h b/lib/flow.h index dcb5bb0..77d4609 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -232,6 +232,8 @@ void flow_set_mpls_bos(struct flow *, int idx, uint8_t stack); void flow_set_mpls_lse(struct flow *, int idx, ovs_be32 lse); void flow_compose(struct dp_packet *, const struct flow *); +void flow_apply_field(void *dst_field, void *dst_wc, const void *src_field, + const void *src_wc, size_t size); static inline uint64_t flow_get_xreg(const struct flow *flow, int idx) @@ -327,6 +329,9 @@ struct flow_wildcards { #define WC_UNMASK_FIELD(WC, FIELD) \ memset(&(WC)->masks.FIELD, 0, sizeof (WC)->masks.FIELD) +bool flow_wildcard_is_fully_masked(void *field, size_t len); +bool flow_wildcard_is_fully_unmasked(void *field, size_t len); + void flow_wildcards_init_catchall(struct flow_wildcards *); void flow_wildcards_init_for_packet(struct flow_wildcards *, -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC HSA 4/4] ofprot-dpif-hsa: Implement HSA prototype.
This commit implements a prototype of Header Space Analysis of the OVS OpenFlow table. What It Does 1. Dump all OpenFlow rules from a specified bridge. 2. Generate all-unmasked header space with specified input port number and start looking up from default starting table. 3. Traverse through all possible processing pipelines (i.e. OpenFlow rules cascaded with resubmit action) and conduct leak check or loop check. 3.1 A leak check prints all possible output ports with the actual input header space format. 3.2 A loop check checks if an OpenFlow rule is matched multiple time in the processing pipeline. It will tell if the loop is finite or infinite and print the loop path. How To Use -- ovs-appctl hsa/detect-leak ovs-appctl hsa/detect-loop Limitation -- - Only support a very limited set of OFPACT_* actions. And extending to accept more actions can be very complicated. - Single thread implementation, can take 20+ mins to run on a production setup with ~100 OpenFlow rules. Signed-off-by: Alex Wang --- ofproto/automake.mk|2 + ofproto/ofproto-dpif-hsa.c | 1572 ofproto/ofproto-dpif-hsa.h | 21 + ofproto/ofproto-dpif.c |2 + 4 files changed, 1597 insertions(+) create mode 100644 ofproto/ofproto-dpif-hsa.c create mode 100644 ofproto/ofproto-dpif-hsa.h diff --git a/ofproto/automake.mk b/ofproto/automake.mk index 0058ff3..9a4e51f 100644 --- a/ofproto/automake.mk +++ b/ofproto/automake.mk @@ -28,6 +28,8 @@ ofproto_libofproto_la_SOURCES = \ ofproto/ofproto.h \ ofproto/ofproto-dpif.c \ ofproto/ofproto-dpif.h \ + ofproto/ofproto-dpif-hsa.c \ + ofproto/ofproto-dpif-hsa.h \ ofproto/ofproto-dpif-ipfix.c \ ofproto/ofproto-dpif-ipfix.h \ ofproto/ofproto-dpif-mirror.c \ diff --git a/ofproto/ofproto-dpif-hsa.c b/ofproto/ofproto-dpif-hsa.c new file mode 100644 index 000..7065c9b --- /dev/null +++ b/ofproto/ofproto-dpif-hsa.c @@ -0,0 +1,1572 @@ +/* + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "ofproto/ofproto-dpif-hsa.h" +#include "ofproto/ofproto-provider.h" + +#include "dynamic-string.h" +#include "flow.h" +#include "hash.h" +#include "hmap.h" +#include "list.h" +#include "match.h" +#include "hsa-match.h" +#include "meta-flow.h" +#include "nx-match.h" +#include "ofproto.h" +#include "ofp-actions.h" +#include "sort.h" +#include "unixctl.h" +#include "util.h" +#include "openvswitch/vlog.h" + +VLOG_DEFINE_THIS_MODULE(hsa); + +/* Supported operations. */ +enum operation_types { +HSA_LEAK_DETECT, +HSA_LOOP_DETECT +}; + +/* Always starts the analysis from OpenFlow table 0. */ +#define TABLE_DEFAULT 0 +/* Default indent level for output. */ +#define INDENT_DEFAULT 0 + +/* Rule used for head space analysis. */ +struct hsa_rule { +struct ovs_list node; /* In owning table's 'rules'. */ + +uint8_t table_id; /* OpenFlow table id. */ +int prio; /* Priority. */ +struct match match; /* Flow and wildcards. */ +const struct rule_actions *actions;/* Actions. */ +struct hsbm hsbm; /* Compressed bit map representation of */ +/* match. */ +}; + +/* Records a previously matched rule's match. */ +struct hs_constraint { +struct ovs_list list_node; +struct match match; +}; + +/* Rule table for header space analysis. */ +struct hsa_table { +struct ovs_list rules; /* Contains 'struct hsa_rule's. */ +size_t n_rules; /* Number of rules in the table. */ +}; + +/* Macros for the 'move_map' member of 'struct header_space'. */ +#define MOVE_MAP_LEN (sizeof(struct flow) * 8) +#define BIT_UNSET 0x +#define BIT_SET0xFFFE + +/* Representation of Header Space. */ +struct header_space { +struct ovs_list list_node; + +/* Difference between match_hs and match_src. + * + *- match_hs represents the input header space as it is transformed by + * matched rules and rule actions. + * + *- match_src represents the input header space that will result in + * the match_hs. + */ +struct match match_hs; /* Current header space. */ +struct match match_src; /* Input header
Re: [ovs-dev] [PATCH 3/9] tunneling: Fix location of GRE checksums.
LGTM Acked-by: Pritesh Kothari > On Mar 30, 2015, at 3:14 PM, Jesse Gross wrote: > > The GRE checksum is a 16 bit field stored in a 32 bit option (the > rest is reserved). The current code treats the checksum as a 32-bit > field and places it in the right place for little endian systems but > not big endian. This fixes the problem by storing the 16 bit field > directly. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/9] tunneling: Use flow flag for GRE checksum calculation.
LGTM Acked-by: Pritesh Kothari will get to rest of series as time permits. > On Mar 30, 2015, at 3:14 PM, Jesse Gross wrote: > > The indication to calculate the GRE checksum is currently the port > config rather than the tunnel flow. Currently there is a one to one > mapping between the two so there is no difference. However, the > kernel datapath must use the flow and it is also potentially more > flexible, so this switches how we decide whether to calculate the > checksum. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Use alternate name for udp_sock_create() backport
On 03/30/15 at 09:29am, Jesse Gross wrote: > On Mon, Mar 30, 2015 at 3:27 AM, Thomas Graf wrote: > > Account for kernels which provide udp_sock_create() in an > > insufficient version. > > > > Avoids the following error when inserting openvswitch.ko on > > respective kernels: > > > > openvswitch: exports duplicate symbol udp_sock_create (owned by udp_tunnel) > > > > Fixes: eb6eebd28 ("datapath: Account for "udp: Add udp_sock_create for UDP > > tunnels to open listener socket") > > Signed-off-by: Thomas Graf > > Cc: Jesse Gross > > Acked-by: Jesse Gross Thanks. Pushed. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn-nbctl: Allow names that look like UUIDs.
This patch fixes handling of logical switch and logical port names that look like a UUID. Previously, if the argument looked like a UUID, it would only do a UUID lookup for a match. After this change, if the UUID lookup fails, it will treat it as a name and look it up that way. Signed-off-by: Russell Bryant --- ovn/ovn-nbctl.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c index eade156..b470e9f 100644 --- a/ovn/ovn-nbctl.c +++ b/ovn/ovn-nbctl.c @@ -91,7 +91,9 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const char *id) is_uuid = true; lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, &lswitch_uuid); -} else { +} + +if (!lswitch) { const struct nbrec_logical_switch *iter; NBREC_LOGICAL_SWITCH_FOR_EACH(iter, nb_ctx->idl) { @@ -225,7 +227,9 @@ lport_by_name_or_uuid(struct nbctl_context *nb_ctx, const char *id) if (uuid_from_string(&lport_uuid, id)) { is_uuid = true; lport = nbrec_logical_port_get_for_uuid(nb_ctx->idl, &lport_uuid); -} else { +} + +if (!lport) { NBREC_LOGICAL_PORT_FOR_EACH(lport, nb_ctx->idl) { if (!strcmp(lport->name, id)) { break; -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] question about availability of Centos 7.1 kernel support
Hi, Can anyone please let me know if Centos 7.1 (3.10.0-229) kernel supported in master branch? I'm currently based off branch-2.3. If this is already available, what would be the commit hash(es) to be picked up? Thanks, Sabya ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vxlan: fix kernel crash with vxlan gso
>> tcp flows with gso between two VMs in diffrent host, go through vxlan >> tunnel, cause kernel crash. >> >> Signed-off-by: caochengrong >> Signed-off-by: Arika Chen > >What OVS and host kernel version is this for? I don't think this should be >necessary, at least on master. vxlan_xmit_skb() calls >udp_tunnel_handle_offloads(), >which calls ip_tunnel_handle_offloads(), which calls >skb_reset_inner_headers(). Thank you for your reply, The OVS version is the master and kernel version is 3.0.93-0.8-xen and the crash stack as follow: [ 6006.808053] [] skb_segment+0x22c/0x670 [ 6006.808053] [] tcp_tso_segment+0x108/0x2a0 [ 6006.808053] [] inet_gso_segment+0xf1/0x270 [ 6006.808053] [] skb_gso_segment+0x13a/0x310 [ 6006.808053] [] rpl_skb_gso_segment+0xb9/0xd0 [openvswitch] [ 6006.808053] [] tnl_skb_gso_segment+0x1b3/0x2a0 [openvswitch] [ 6006.808053] [] rpl_ip_local_out+0x9b/0xe0 [openvswitch] [ 6006.808053] [] rpl_iptunnel_xmit+0x172/0x1e0 [openvswitch] [ 6006.808053] [] vxlan_tnl_send+0x1ed/0x340 [openvswitch] [ 6006.808053] [] ovs_vport_send+0xb/0x50 [openvswitch] [ 6006.808053] [] do_execute_actions+0x3bd/0x620 [openvswitch] [ 6006.808053] [] ovs_execute_actions+0x47/0x130 [openvswitch] [ 6006.808053] [] ovs_dp_process_packet+0xa3/0x170 [openvswitch] [ 6006.808053] [] ovs_vport_receive+0x80/0xa0 [openvswitch] [ 6006.808053] [] netdev_frame_hook+0x2c/0x50 [openvswitch] [ 6006.808053] [] __netif_receive_skb+0x2fd/0x7f0 [ 6006.808053] [] process_backlog+0xc7/0x220 [ 6006.808053] [] net_rx_action+0x13e/0x2a0 [ 6006.808053] [] __do_softirq+0xff/0x240 [ 6006.808053] [] call_softirq+0x1c/0x30 [ 6006.808053] [] do_softirq+0x95/0xd0 [ 6006.808053] [] netif_rx_ni+0x19/0x20 [ 6006.808053] [] net_tx_action+0x90e/0x14a0 [netbk] [ 6006.808053] [] netbk_action_thread+0x89/0x1d0 [netbk] [ 6006.808053] [] kthread+0x96/0xa0 [ 6006.808053] [] kernel_thread_helper+0x4/0x10 [ 6006.808053] Code: 50 19 c0 49 89 70 58 41 c6 40 4c 04 83 e0 fc 83 c0 08 41 88 40 4d c3 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 48 89 f8 89 d1 a4 c3 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 20 48 83 ea 20 4c 8b [ 6006.808053] RIP [] memcpy+0x5/0x120 [ 6006.808053] RSP And after we add the patch, vm send tcp packets is ok. So what you think about ? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH ovn 0/5] Implement matching expressions for OVN.
The syntax of matching expressions supported here is documented in ovn(5), which I've made available (along with the rest of the OVS docs for this branch) at http://benpfaff.org/~blp/dist-docs, e.g. specifically http://benpfaff.org/~blp/dist-docs/ovn.5.pdf Ben Pfaff (5): meta-flow: Add convenience members to union mf_subvalue. lex: New lexical analyzer module for use in OVN. util: Add more bitwise operations. meta-flow: Add new functions for subvalues. expr: New module for Boolean expressions on fields, for use in OVN. lib/learn.c|6 +- lib/meta-flow.c| 81 +- lib/meta-flow.h| 29 + lib/util.c | 139 +++- lib/util.h |8 +- ovn/TODO | 30 - ovn/automake.mk|3 + ovn/expr.c | 2315 ovn/expr.h | 367 + ovn/lex.c | 697 ovn/lex.h | 109 +++ ovn/ovn.xml| 355 +--- tests/automake.mk |6 +- tests/ovn.at | 348 tests/test-ovn.c | 1294 + tests/testsuite.at |1 + 16 files changed, 5643 insertions(+), 145 deletions(-) create mode 100644 ovn/expr.c create mode 100644 ovn/expr.h create mode 100644 ovn/lex.c create mode 100644 ovn/lex.h create mode 100644 tests/ovn.at create mode 100644 tests/test-ovn.c -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH ovn 1/5] meta-flow: Add convenience members to union mf_subvalue.
This makes access to the least-significant bits more convenient. This commit simplifies a few existing cases; later commits will make more use of this feature. Signed-off-by: Ben Pfaff --- lib/learn.c | 6 ++ lib/meta-flow.h | 16 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/learn.c b/lib/learn.c index 99d56e6..7ed7884 100644 --- a/lib/learn.c +++ b/lib/learn.c @@ -148,8 +148,7 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow, case NX_LEARN_DST_OUTPUT: if (spec->n_bits <= 16 || is_all_zeros(value.u8, sizeof value - 2)) { -ovs_be16 *last_be16 = &value.be16[ARRAY_SIZE(value.be16) - 1]; -ofp_port_t port = u16_to_ofp(ntohs(*last_be16)); +ofp_port_t port = u16_to_ofp(ntohll(value.integer)); if (ofp_to_u16(port) < ofp_to_u16(OFPP_MAX) || port == OFPP_IN_PORT @@ -211,8 +210,7 @@ learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec) } s = arrow; } else { -ovs_be64 *last_be64 = &imm.be64[ARRAY_SIZE(imm.be64) - 1]; -*last_be64 = htonll(strtoull(s, (char **) &s, 0)); +imm.integer = htonll(strtoull(s, (char **) &s, 0)); } if (strncmp(s, "->", 2)) { diff --git a/lib/meta-flow.h b/lib/meta-flow.h index 0e09036..de02d1a 100644 --- a/lib/meta-flow.h +++ b/lib/meta-flow.h @@ -1567,10 +1567,26 @@ struct mf_subfield { * value" contains NXM_OF_VLAN_TCI[0..11], then one could access the * corresponding data in value.be16[7] as the bits in the mask htons(0xfff). */ union mf_subvalue { +/* Access to full data. */ uint8_t u8[16]; ovs_be16 be16[8]; ovs_be32 be32[4]; ovs_be64 be64[2]; + +/* Convenient access to just least-significant bits in various forms. */ +struct { +ovs_be64 dummy_integer; +ovs_be64 integer; +}; +struct { +uint8_t dummy_mac[10]; +uint8_t mac[6]; +}; +struct { +ovs_be32 dummy_ipv4[3]; +ovs_be32 ipv4; +}; +struct in6_addr ipv6; }; BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue)); -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH ovn 3/5] util: Add more bitwise operations.
To be used in upcoming commits. Signed-off-by: Ben Pfaff --- lib/util.c | 139 + lib/util.h | 8 +++- 2 files changed, 139 insertions(+), 8 deletions(-) diff --git a/lib/util.c b/lib/util.c index bcf7700..3293ca4 100644 --- a/lib/util.c +++ b/lib/util.c @@ -1284,9 +1284,9 @@ bitwise_is_all_zeros(const void *p_, unsigned int len, unsigned int ofs, return true; } -/* Scans the bits in 'p' that have bit offsets 'start' through 'end' - * (inclusive) for the first bit with value 'target'. If one is found, returns - * its offset, otherwise 'end'. 'p' is 'len' bytes long. +/* Scans the bits in 'p' that have bit offsets 'start' (inclusive) through + * 'end' (exclusive) for the first bit with value 'target'. If one is found, + * returns its offset, otherwise 'end'. 'p' is 'len' bytes long. * * If you consider all of 'p' to be a single unsigned integer in network byte * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit @@ -1297,21 +1297,48 @@ bitwise_is_all_zeros(const void *p_, unsigned int len, unsigned int ofs, * start <= end */ unsigned int -bitwise_scan(const void *p_, unsigned int len, bool target, unsigned int start, +bitwise_scan(const void *p, unsigned int len, bool target, unsigned int start, unsigned int end) { -const uint8_t *p = p_; unsigned int ofs; for (ofs = start; ofs < end; ofs++) { -bool bit = (p[len - (ofs / 8 + 1)] & (1u << (ofs % 8))) != 0; -if (bit == target) { +if (bitwise_get_bit(p, len, ofs) == target) { break; } } return ofs; } +/* Scans the bits in 'p' that have bit offsets 'start' (inclusive) through + * 'end' (exclusive) for the first bit with value 'target', in reverse order. + * If one is found, returns its offset, otherwise 'end'. 'p' is 'len' bytes + * long. + * + * If you consider all of 'p' to be a single unsigned integer in network byte + * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit + * with value 1 in p[len - 1], bit 1 is the bit with value 2, bit 2 is the bit + * with value 4, ..., bit 8 is the bit with value 1 in p[len - 2], and so on. + * + * To scan an entire bit array in reverse order, specify start == len * 8 - 1 + * and end == -1, in which case the return value is nonnegative if successful + * and -1 if no 'target' match is found. + * + * Required invariant: + * start >= end + */ +int +bitwise_rscan(const void *p, int len, bool target, int start, int end) +{ +int ofs; + +for (ofs = start; ofs > end; ofs--) { +if (bitwise_get_bit(p, len, ofs) == target) { +break; +} +} +return ofs; +} /* Copies the 'n_bits' low-order bits of 'value' into the 'n_bits' bits * starting at bit 'dst_ofs' in 'dst', which is 'dst_len' bytes long. @@ -1361,6 +1388,104 @@ bitwise_get(const void *src, unsigned int src_len, n_bits); return ntohll(value); } + +/* Returns the value of the bit with offset 'ofs' in 'ofs', which is 'len' + * bytes long. + * + * If you consider all of 'src' to be a single unsigned integer in network byte + * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit + * with value 1 in src[len - 1], bit 1 is the bit with value 2, bit 2 is the + * bit with value 4, ..., bit 8 is the bit with value 1 in src[len - 2], and so + * on. + * + * Required invariants: + * ofs < len * 8 + */ +bool +bitwise_get_bit(const void *src_, unsigned int len, unsigned int ofs) +{ +const uint8_t *src = src_; + +return (src[len - (ofs / 8 + 1)] & (1u << (ofs % 8))) != 0; +} + +/* Sets the bit with offset 'ofs' in 'ofs', which is 'len' bytes long, to 0. + * + * If you consider all of 'src' to be a single unsigned integer in network byte + * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit + * with value 1 in src[len - 1], bit 1 is the bit with value 2, bit 2 is the + * bit with value 4, ..., bit 8 is the bit with value 1 in src[len - 2], and so + * on. + * + * Required invariants: + * ofs < len * 8 + */ +void +bitwise_put0(void *dst_, unsigned int len, unsigned int ofs) +{ +uint8_t *dst = dst_; + +dst[len - (ofs / 8 + 1)] &= ~(1u << (ofs % 8)); +} + +/* Sets the bit with offset 'ofs' in 'ofs', which is 'len' bytes long, to 1. + * + * If you consider all of 'dst' to be a single unsigned integer in network byte + * order, then bit N is the bit with value 2**N. That is, bit 0 is the bit + * with value 1 in dst[len - 1], bit 1 is the bit with value 2, bit 2 is the + * bit with value 4, ..., bit 8 is the bit with value 1 in dst[len - 2], and so + * on. + * + * Required invariants: + * ofs < len * 8 + */ +void +bitwise_put1(void *dst_, unsigned int len, unsigned int ofs) +{ +uint8_t *dst = dst_; + +dst[len - (ofs / 8 + 1)] |= 1u << (ofs % 8); +} + +/* Sets the bit with offset 'ofs' in 'ofs', which is 'len' bytes long, to 'b'. +
[ovs-dev] [PATCH ovn 2/5] lex: New lexical analyzer module for use in OVN.
I'm determined not to let the terrible style of pseudo-parsing we have in OVS leak into OVN. Here's the first step. Signed-off-by: Ben Pfaff --- ovn/TODO | 5 - ovn/automake.mk| 3 + ovn/lex.c | 697 + ovn/lex.h | 109 + tests/automake.mk | 6 +- tests/ovn.at | 95 tests/test-ovn.c | 119 + tests/testsuite.at | 1 + 8 files changed, 1028 insertions(+), 7 deletions(-) create mode 100644 ovn/lex.c create mode 100644 ovn/lex.h create mode 100644 tests/ovn.at create mode 100644 tests/test-ovn.c diff --git a/ovn/TODO b/ovn/TODO index 43a867c..d91c3cf 100644 --- a/ovn/TODO +++ b/ovn/TODO @@ -19,11 +19,6 @@ Probably should be defined so that the data structure is also useful for references to fields in action parsing. -** Lexical analysis. - - Probably should be defined so that the lexer can be reused for - parsing actions. - ** Parsing into syntax tree. ** Semantic checking against variable definitions. diff --git a/ovn/automake.mk b/ovn/automake.mk index 37b0ca6..940340c 100644 --- a/ovn/automake.mk +++ b/ovn/automake.mk @@ -74,6 +74,9 @@ SUFFIXES += .xml $(AM_V_GEN)$(run_python) $(srcdir)/build-aux/xml2nroff \ --version=$(VERSION) $< > $@.tmp && mv $@.tmp $@ +lib_LTLIBRARIES += lib/libovn.la +lib_libovn_la_SOURCES = ovn/lex.c ovn/lex.h + EXTRA_DIST += ovn/TODO # ovn IDL diff --git a/ovn/lex.c b/ovn/lex.c new file mode 100644 index 000..a837f7c --- /dev/null +++ b/ovn/lex.c @@ -0,0 +1,697 @@ +/* + * Copyright (c) 2015 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "lex.h" +#include +#include +#include +#include "dynamic-string.h" +#include "json.h" +#include "util.h" + +/* Initializes 'token'. */ +void +lex_token_init(struct lex_token *token) +{ +token->type = LEX_T_END; +token->s = NULL; +} + +/* Frees memory owned by 'token'. */ +void +lex_token_destroy(struct lex_token *token) +{ +free(token->s); +} + +/* Exchanges 'a' and 'b'. */ +void +lex_token_swap(struct lex_token *a, struct lex_token *b) +{ +struct lex_token tmp = *a; +*a = *b; +*b = tmp; +} + +/* lex_token_format(). */ + +static size_t +lex_token_n_zeros(enum lex_format format) +{ +switch (format) { +case LEX_F_DECIMAL: return offsetof(union mf_subvalue, integer); +case LEX_F_HEXADECIMAL: return 0; +case LEX_F_IPV4:return offsetof(union mf_subvalue, ipv4); +case LEX_F_IPV6:return offsetof(union mf_subvalue, ipv6); +case LEX_F_ETHERNET:return offsetof(union mf_subvalue, mac); +default: OVS_NOT_REACHED(); +} +} + +/* Returns the effective format for 'token', that is, the format in which it + * should actually be printed. This is ordinarily the same as 'token->format', + * but it's always possible that someone sets up a token with a format that + * won't work for a value, e.g. 'token->value' is wider than 32 bits but the + * format is LEX_F_IPV4. (The lexer itself won't do that; this is an attempt + * to avoid confusion in the future.) */ +static enum lex_format +lex_token_get_format(const struct lex_token *token) +{ +size_t n_zeros = lex_token_n_zeros(token->format); +return (is_all_zeros(&token->value, n_zeros) +&& (token->type != LEX_T_MASKED_INTEGER +|| is_all_zeros(&token->mask, n_zeros)) +? token->format +: LEX_F_HEXADECIMAL); +} + +static void +lex_token_format_value(const union mf_subvalue *value, + enum lex_format format, struct ds *s) +{ +switch (format) { +case LEX_F_DECIMAL: +ds_put_format(s, "%"PRIu64, ntohll(value->integer)); +break; + +case LEX_F_HEXADECIMAL: +mf_format_subvalue(value, s); +break; + +case LEX_F_IPV4: +ds_put_format(s, IP_FMT, IP_ARGS(value->ipv4)); +break; + +case LEX_F_IPV6: +print_ipv6_addr(s, &value->ipv6); +break; + +case LEX_F_ETHERNET: +ds_put_format(s, ETH_ADDR_FMT, ETH_ADDR_ARGS(value->mac)); +break; + +default: +OVS_NOT_REACHED(); +} + +} + +static void +lex_token_format_masked_integer(const struct lex_token *token, struct ds *s) +{ +enum lex_format format = lex_token_get_format(token); + +lex_token_format_value(&token->value, format, s); +ds_put_char(s, '/'); + +const union mf
[ovs-dev] [PATCH ovn 4/5] meta-flow: Add new functions for subvalues.
To be first used in upcoming commits. Signed-off-by: Ben Pfaff --- lib/meta-flow.c | 81 - lib/meta-flow.h | 13 + 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 8058e07..f810376 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc. + * Copyright (c) 2011, 2012, 2013, 2014, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,6 +95,69 @@ nxm_init(void) pthread_once(&once, nxm_do_init); } +/* Consider the two value/mask pairs 'a_value/a_mask' and 'b_value/b_mask' as + * restrictions on a field's value. Then, this function initializes + * 'dst_value/dst_mask' such that it combines the restrictions of both pairs. + * This is not always possible, i.e. if one pair insists on a value of 0 in + * some bit and the other pair insists on a value of 1 in that bit. This + * function returns false in a case where the combined restriction is + * impossible (in which case 'dst_value/dst_mask' is not fully initialized), + * true otherwise. */ +bool +mf_subvalue_intersect(const union mf_subvalue *a_value, + const union mf_subvalue *a_mask, + const union mf_subvalue *b_value, + const union mf_subvalue *b_mask, + union mf_subvalue *dst_value, + union mf_subvalue *dst_mask) +{ +for (int i = 0; i < ARRAY_SIZE(a_value->be64); i++) { +ovs_be64 av = a_value->be64[i]; +ovs_be64 am = a_mask->be64[i]; +ovs_be64 bv = b_value->be64[i]; +ovs_be64 bm = b_mask->be64[i]; +ovs_be64 *dv = &dst_value->be64[i]; +ovs_be64 *dm = &dst_mask->be64[i]; + +if ((av ^ bv) & (am & bm)) { +return false; +} +*dv = av | bv; +*dm = am | bm; +} +return true; +} + +/* Returns the "number of bits" in 'v', e.g. 1 if only the lowest-order bit is + * set, 2 if the second-lowest-order bit is set, and so on. */ +int +mf_subvalue_width(const union mf_subvalue *v) +{ +return 1 + bitwise_rscan(v, sizeof *v, true, sizeof *v * 8 - 1, -1); +} + +/* For positive 'n', shifts the bits in 'value' 'n' bits to the left, and for + * negative 'n', shifts the bits '-n' bits to the right. */ +void +mf_subvalue_shift(union mf_subvalue *value, int n) +{ +if (n) { +union mf_subvalue tmp; +memset(&tmp, 0, sizeof tmp); + +if (n > 0 && n < 8 * sizeof tmp) { +bitwise_copy(value, sizeof *value, 0, + &tmp, sizeof tmp, n, + 8 * sizeof tmp - n); +} else if (n < 0 && n > -8 * sizeof tmp) { +bitwise_copy(value, sizeof *value, -n, + &tmp, sizeof tmp, 0, + 8 * sizeof tmp + n); +} +*value = tmp; +} +} + /* Returns true if 'wc' wildcards all the bits in field 'mf', false if 'wc' * specifies at least one bit in the field. * @@ -2234,6 +2297,22 @@ mf_write_subfield(const struct mf_subfield *sf, const union mf_subvalue *x, mf_set(field, &value, &mask, match); } +/* 'v' and 'm' correspond to values of 'field'. This function copies them into + * 'match' in the correspond positions. */ +void +mf_mask_subfield(const struct mf_field *field, + const union mf_subvalue *v, + const union mf_subvalue *m, + struct match *match) +{ +union mf_value value, mask; + +mf_get(field, match, &value, &mask); +bitwise_copy(v, sizeof *v, 0, &value, field->n_bytes, 0, field->n_bits); +bitwise_copy(m, sizeof *m, 0, &mask, field->n_bytes, 0, field->n_bits); +mf_set(field, &value, &mask, match); +} + /* Initializes 'x' to the value of 'sf' within 'flow'. 'sf' must be valid for * reading 'flow', e.g. as checked by mf_check_src(). */ void diff --git a/lib/meta-flow.h b/lib/meta-flow.h index de02d1a..8e1b932 100644 --- a/lib/meta-flow.h +++ b/lib/meta-flow.h @@ -1590,6 +1590,15 @@ union mf_subvalue { }; BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue)); +bool mf_subvalue_intersect(const union mf_subvalue *a_value, + const union mf_subvalue *a_mask, + const union mf_subvalue *b_value, + const union mf_subvalue *b_mask, + union mf_subvalue *dst_value, + union mf_subvalue *dst_mask); +int mf_subvalue_width(const union mf_subvalue *); +void mf_subvalue_shift(union mf_subvalue *, int n); + /* Finding mf_fields. */ const struct mf_field *mf_from_name(const char *name); @@ -1650,6 +1659,10 @@ void mf_write_subfield_flow(const struct mf_subfield *, const union mf_subvalue
Re: [ovs-dev] [PATCH] ovn-nbctl: Allow names that look like UUIDs.
On Mon, Mar 30, 2015 at 10:07:04PM -0400, Russell Bryant wrote: > This patch fixes handling of logical switch and logical port names > that look like a UUID. Previously, if the argument looked like a > UUID, it would only do a UUID lookup for a match. After this change, > if the UUID lookup fails, it will treat it as a name and look it up > that way. > > Signed-off-by: Russell Bryant Good catch! Thanks, pushed. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC HSA 1/4] ovs_list: Extend list functions.
On Mon, Mar 30, 2015 at 03:46:26PM -0700, Alex Wang wrote: > This commit adds function that allows the appending of one list > content to the other. Also, it adds functions which allows list > to be sorted. > > Signed-off-by: Alex Wang I always like new basic algorithms! > +/* Appends 'src''s content to the end of 'dst'. After 'list_join', 'src' > + * becomes an empty list. */ > +static inline void > +list_join(struct ovs_list *dst, struct ovs_list *src) > +{ > +if (list_is_empty(src)) { > +return; > +} > + > +list_splice(dst, list_front(src), src); > +} I think that list_join() can be simplified to just: list_splice(dst, src->next, src); because if 'src' is empty then src->next == next and list_splice() will become a no-op because of its test for first == last. > +/* Swaps the two adjacent list elements. 'ahead' must be ahead of 'after'. */ > +static inline void > +list_swap_adjacent(struct ovs_list *ahead, struct ovs_list *after) > +{ > +ahead->next = after->next; > +ahead->next->prev = ahead; > +after->prev = ahead->prev; > +ahead->prev->next = after; > +ahead->prev = after; > +after->next = ahead; > +} > + > +/* Swaps the position of two list nodes, 'elem1' and 'elem2'. */ > +static inline void > +list_swap(struct ovs_list *elem1, struct ovs_list *elem2) > +{ > +if (elem1 == elem2) { > +return; > +} else if (elem1->prev == elem2) { > +/* element2 is ahead of element1. */ > +list_swap_adjacent(elem2, elem1); > +} else if (elem2->prev == elem1) { > +/* element1 is ahead of element2. */ > +list_swap_adjacent(elem1, elem2); > +} else { > +const struct ovs_list tmp = *elem1; > + > +list_replace(elem1, elem2); > +list_replace(elem2, &tmp); > +} > +} I think that list_swap() can be simplified to the following, although I have not tested it: /* Exchanges the positions of A and B, which may be in the same list * or different lists. */ void ovs_list_swap(struct ovs_list *a, struct ovs_list *b) { if (a != b) { if (a->next != b) { struct ovs_list *a_next = list_remove(a); struct ovs_list *b_next = list_remove(b); list_insert(b_next, a); list_insert(a_next, b); } else { list_remove(b); list_insert(a, b); } } } > /* Adjusts pointers around 'list' to compensate for 'list' having been moved > * around in memory (e.g. as a consequence of realloc()), with original > * location 'orig'. > @@ -233,6 +281,25 @@ list_back(const struct ovs_list *list_) > return list->prev; > } > > +/* Returns the element at position 'n', assumes the first element is > + * at index 0. The 'list_' must contain at least 'n' elements. */ > +static inline struct ovs_list * > +list_at_position(const struct ovs_list *list_, size_t n) > +{ > +struct ovs_list *list = CONST_CAST(struct ovs_list *, list_); > +struct ovs_list *ret; > +size_t cnt = 0; > + > +for (ret = list->next; ret != list; ret = ret->next) { > +if (cnt++ == n) { > +return ret; > +} > +} I'd probably just write the following two statements as OVS_NOT_REACHED(): > +ovs_assert(false); > + > +return NULL; > +} Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/9] tunneling: Add check for GRE protocol is Ethernet.
It's probably a good idea although it requires a different type of test compared with what we already have since the GRE protocol doesn't appear in the flow. Here's what I came up with: diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index 6e1c0c1..ee17a2f 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -75,5 +75,21 @@ AT_CHECK([tail -1 stdout], [0], [Datapath actions: tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x40),gre((flags=0x20,proto=0x6558),key=0x1c8)),out_port(100)) ]) +dnl Check decapsulation of GRE packet +AT_CHECK([ovs-appctl netdev-dummy/receive p0 '001b213cac30001b213cab640800457e79464000402f99080101025c010102582000655801c8fe71d883724fbeb6f4e1494a08004554ba20400184861e011e024227e75400030af31955f2650100101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) +ovs-appctl time/warp 1000 + +AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 3'], [0], [dnl + port 3: rx pkts=1, bytes=98, drop=0, errs=0, frame=0, over=0, crc=0 +]) + +dnl Check GRE only accepts encapsulated Ethernet frames +AT_CHECK([ovs-appctl netdev-dummy/receive p0 '001b213cac30001b213cab640800457e79464000402f99080101025c01010258280001c8fe71d883724fbeb6f4e1494a08004554ba20400184861e011e024227e75400030af31955f2650100101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637']) +ovs-appctl time/warp 1000 + +AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 3'], [0], [dnl + port 3: rx pkts=1, bytes=98, drop=0, errs=0, frame=0, over=0, crc=0 +]) + OVS_VSWITCHD_STOP AT_CLEANUP On Mon, Mar 30, 2015 at 3:42 PM, Pritesh Kothari (pritkoth) wrote: > should a test case be added to catch this ? > > Regards, > Pritesh > >> On Mar 30, 2015, at 3:14 PM, Jesse Gross wrote: >> >> +if (greh->protocol != htons(ETH_TYPE_TEB)) { >> +return -EINVAL; >> +} >> + > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Editing Ip headers in datapath.c
I am trying to edit the Ip source and destination address in void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb) in datapath.c file. I am able to edit mac headers successfully using memcpy(eth_hdr(skb), str, 2) // str contains my target data I tried a similar approach with Ip headers as well memcpy(ip_hdr(skb), str, 2) But it isnt giving the expected results, in fact the rewriting isnt getting executed. Could anyone please help me with the proper code segment to implement the above?? Thanks in advance! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC HSA 1/4] ovs_list: Extend list functions.
Thx a lot for the suggested changes~, I really did it the hard way! The changes are simple and clean! really like it, Adopt them all, Thanks, Alex Wang, On Mon, Mar 30, 2015 at 9:19 PM, Ben Pfaff wrote: > On Mon, Mar 30, 2015 at 03:46:26PM -0700, Alex Wang wrote: > > This commit adds function that allows the appending of one list > > content to the other. Also, it adds functions which allows list > > to be sorted. > > > > Signed-off-by: Alex Wang > > I always like new basic algorithms! > > > +/* Appends 'src''s content to the end of 'dst'. After 'list_join', > 'src' > > + * becomes an empty list. */ > > +static inline void > > +list_join(struct ovs_list *dst, struct ovs_list *src) > > +{ > > +if (list_is_empty(src)) { > > +return; > > +} > > + > > +list_splice(dst, list_front(src), src); > > +} > > I think that list_join() can be simplified to just: > list_splice(dst, src->next, src); > because if 'src' is empty then src->next == next and list_splice() > will become a no-op because of its test for first == last. > > > +/* Swaps the two adjacent list elements. 'ahead' must be ahead of > 'after'. */ > > +static inline void > > +list_swap_adjacent(struct ovs_list *ahead, struct ovs_list *after) > > +{ > > +ahead->next = after->next; > > +ahead->next->prev = ahead; > > +after->prev = ahead->prev; > > +ahead->prev->next = after; > > +ahead->prev = after; > > +after->next = ahead; > > +} > > + > > +/* Swaps the position of two list nodes, 'elem1' and 'elem2'. */ > > +static inline void > > +list_swap(struct ovs_list *elem1, struct ovs_list *elem2) > > +{ > > +if (elem1 == elem2) { > > +return; > > +} else if (elem1->prev == elem2) { > > +/* element2 is ahead of element1. */ > > +list_swap_adjacent(elem2, elem1); > > +} else if (elem2->prev == elem1) { > > +/* element1 is ahead of element2. */ > > +list_swap_adjacent(elem1, elem2); > > +} else { > > +const struct ovs_list tmp = *elem1; > > + > > +list_replace(elem1, elem2); > > +list_replace(elem2, &tmp); > > +} > > +} > > I think that list_swap() can be simplified to the following, although I > have not tested it: > > /* Exchanges the positions of A and B, which may be in the same list > * or different lists. */ > void > ovs_list_swap(struct ovs_list *a, struct ovs_list *b) > { > if (a != b) > { > if (a->next != b) > { > struct ovs_list *a_next = list_remove(a); > struct ovs_list *b_next = list_remove(b); > list_insert(b_next, a); > list_insert(a_next, b); > } > else > { > list_remove(b); > list_insert(a, b); > } > } > } > > > /* Adjusts pointers around 'list' to compensate for 'list' having been > moved > > * around in memory (e.g. as a consequence of realloc()), with original > > * location 'orig'. > > @@ -233,6 +281,25 @@ list_back(const struct ovs_list *list_) > > return list->prev; > > } > > > > +/* Returns the element at position 'n', assumes the first element is > > + * at index 0. The 'list_' must contain at least 'n' elements. */ > > +static inline struct ovs_list * > > +list_at_position(const struct ovs_list *list_, size_t n) > > +{ > > +struct ovs_list *list = CONST_CAST(struct ovs_list *, list_); > > +struct ovs_list *ret; > > +size_t cnt = 0; > > + > > +for (ret = list->next; ret != list; ret = ret->next) { > > +if (cnt++ == n) { > > +return ret; > > +} > > +} > > I'd probably just write the following two statements as OVS_NOT_REACHED(): > > +ovs_assert(false); > > + > > +return NULL; > > +} > > Thanks, > > Ben. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev