from:"solomon"

[ovs-dev] [PATCH] openvswitch: meter: Remove unnecessary int

2022-04-18 Thread Solomon Tan via dev

This patch addresses the checkpatch.pl warning that long long is
preferred over long long int.

Signed-off-by: Solomon Tan 
---
 net/openvswitch/meter.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
index 04a060ac7fdf..a790920c11d6 100644
--- a/net/openvswitch/meter.c
+++ b/net/openvswitch/meter.c
@@ -592,8 +592,8 @@ static int ovs_meter_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
 bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
   struct sw_flow_key *key, u32 meter_id)
 {
-   long long int now_ms = div_u64(ktime_get_ns(), 1000 * 1000);
-   long long int long_delta_ms;
+   long long now_ms = div_u64(ktime_get_ns(), 1000 * 1000);
+   long long long_delta_ms;
struct dp_meter_band *band;
struct dp_meter *meter;
int i, band_exceeded_max = -1;
@@ -622,7 +622,7 @@ bool ovs_meter_execute(struct datapath *dp, struct sk_buff 
*skb,
/* Make sure delta_ms will not be too large, so that bucket will not
 * wrap around below.
 */
-   delta_ms = (long_delta_ms > (long long int)meter->max_delta_t)
+   delta_ms = (long_delta_ms > (long long)meter->max_delta_t)
   ? meter->max_delta_t : (u32)long_delta_ms;

/* Update meter statistics.
@@ -645,7 +645,7 @@ bool ovs_meter_execute(struct datapath *dp, struct sk_buff 
*skb,

/* Update all bands and find the one hit with the highest rate. */
for (i = 0; i < meter->n_bands; ++i) {
-   long long int max_bucket_size;
+   long long max_bucket_size;

band = >bands[i];
max_bucket_size = band->burst_size * 1000LL;
--
2.35.3


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

[ovs-dev] achat terrains

2020-08-30 Thread Dahlia SOLOMON


Bonjour,



Nous recherchons pour nos clients Promoteurs, Marchands de Biens et
Investisseurs, des terrains constructibles, des immeubles vides ou
lous, des locaux commerciaux ou industriels.



Toute France mais uniquement dans les grandes villes et leurs
agglomrations. Tous budgets.



Dcrivez-nous votre bien sur notre formulaire decontact
ICIet nous reviendrons vers vous trs rapidement.





Bien Cordialement



O.R.V.A.L.Patrimoine

















cliquez ici pour vous dsabonner.

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

Re: [ovs-dev] [patch v5] conntrack: ignore port for ICMP/ICMPv6 NAT.

2019-06-05 Thread solomon


It looks good to me，thank you.


Darrell Ball wrote:
> From: solomon 
> 
> ICMP/ICMPv6 fails, if the src/dst port is set in a common NAT rule.
> For example:
> actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)
> 
> Fixes: 4cd0481c9e8b ("conntrack: Fix wasted work for ICMP NAT.")
> CC: Darrell Ball 
> Signed-off-by: solomon 
> Signed-off-by: Darrell Ball 
> Co-authored-by: Darrell Ball 
> ---
> 
> v5: Rebase
> v4: Fix corrupted test.
> v3: Add co-author tag.
> v2: Add a test.
> 
>  lib/conntrack.c | 12 
>  tests/system-traffic.at | 48 
>  2 files changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 67c3a58..5f60fea 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2051,14 +2051,18 @@ nat_select_range_tuple(struct conntrack *ct, const 
> struct conn *conn,
>  while (true) {
>  if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
>  nat_conn->rev_key.dst.addr = ct_addr;
> -nat_conn->rev_key.dst.port = htons(port);
> +if (pat_enabled) {
> +nat_conn->rev_key.dst.port = htons(port);
> +}
>  } else {
>  nat_conn->rev_key.src.addr = ct_addr;
> -nat_conn->rev_key.src.port = htons(port);
> +if (pat_enabled) {
> +nat_conn->rev_key.src.port = htons(port);
> +}
>  }
>  
> -bool found = conn_lookup(ct, _conn->rev_key, time_msec(),
> - NULL, NULL);
> +bool found = conn_lookup(ct, _conn->rev_key, time_msec(), NULL,
> + NULL);
>  if (!found) {
>  return true;
>  } else if (pat_enabled && !all_ports_tried) {
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 79d12fe..d23ee89 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -3994,6 +3994,54 @@ 
> tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>  
> +AT_SETUP([conntrack - SNAT with port range using ICMP])
> +dnl Check PAT is not attempted on ICMP packets causing corrupted packets.
> +CHECK_CONNTRACK()
> +CHECK_CONNTRACK_NAT()
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +ADD_NAMESPACES(at_ns0, at_ns1)
> +
> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> +NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> +
> +dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from 
> ns1->ns0.
> +AT_DATA([flows.txt], [dnl
> +in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255:2)),2
> +in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
> +in_port=2,ct_state=+trk,ct_zone=1,action=1
> +dnl
> +dnl ARP
> +priority=100 arp arp_op=1 
> action=move:OXM_OF_ARP_TPA[[]]->NXM_NX_REG2[[]],resubmit(,8),goto_table:10
> +priority=10 arp action=normal
> +priority=0,action=drop
> +dnl
> +dnl MAC resolution table for IP in reg2, stores mac in OXM_OF_PKT_REG0
> +table=8,reg2=0x0a0101f0/0xfff0,action=load:0x8088->OXM_OF_PKT_REG0[[]]
> +table=8,priority=0,action=load:0->OXM_OF_PKT_REG0[[]]
> +dnl ARP responder mac filled in at OXM_OF_PKT_REG0, or 0 for normal action.
> +dnl TPA IP in reg2.
> +dnl Swaps the fields of the ARP message to turn a query to a response.
> +table=10 priority=100 arp xreg0=0 action=normal
> +table=10 
> priority=10,arp,arp_op=1,action=load:2->OXM_OF_ARP_OP[[]],move:OXM_OF_ARP_SHA[[]]->OXM_OF_ARP_THA[[]],move:OXM_OF_PKT_REG0[[0..47]]->OXM_OF_ARP_SHA[[]],move:OXM_OF_ARP_SPA[[]]->OXM_OF_ARP_TPA[[]],move:NXM_NX_REG2[[]]->OXM_OF_ARP_SPA[[]],move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],move:OXM_OF_PKT_REG0[[0..47]]->NXM_OF_ETH_SRC[[]],move:NXM_OF_IN_PORT[[]]->NXM_NX_REG3[[0..15]],load:0->NXM_OF_IN_PORT[[]],output:NXM_NX_REG3[[0..15]]
> +table=10 priority=0 action=drop
> +])
> +
> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +
> +dnl ICMP requests from p0->p1 should work fine.
> +NS_CHECK_EXEC([at_ns0], [ping -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl
> +1 packets transmitted, 1 received, 0% packet loss, time 0ms
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e 
> 's/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/'], [0], [dnl
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.2XX,id=,type=0,code=0),zone=1
> +])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([conntrack - SNAT with port range with exhaustion])
>  CHECK_CONNTRACK()
>  CHECK_CONNTRACK_NAT()
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3]conntrack: ignore port for ICMP/ICMPv6 NAT.

2019-06-01 Thread solomon



ICMP/ICMPv6 fails, if the src/dst port is set in a common NAT rule.
like this:
actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)


Fixes: 4cd0481c9e8b ("conntrack: Fix wasted work for ICMP NAT.")
CC: Darrell Ball 
Signed-off-by: solomon 
Signed-off-by: Darrell Ball 
Co-authored-by: Darrell Ball 
---
 lib/conntrack.c |  8 +--
 tests/system-traffic.at | 51 +
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index d7d48a43a..9d6b8a358 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2039,10 +2039,14 @@ nat_select_range_tuple(struct conntrack *ct, const 
struct conn *conn,
 while (true) {
 if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
 nat_conn->rev_key.dst.addr = ct_addr;
-nat_conn->rev_key.dst.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.dst.port = htons(port);
+}
 } else {
 nat_conn->rev_key.src.addr = ct_addr;
-nat_conn->rev_key.src.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.src.port = htons(port);
+}
 }
 
 uint32_t conn_hash = conn_key_hash(_conn->rev_key,
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 79d12fecd..7cf7d9392 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3994,6 +3994,57 @@ 
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - SNAT with port range using ICMP])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from
+ns1->ns0.
+AT_DATA([flows.txt], [dnl
+in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255:2)),2
+in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
+in_port=2,ct_state=+trk,ct_zone=1,action=1
+dnl
+dnl ARP
+priority=100 arp arp_op=1
+action=move:OXM_OF_ARP_TPA[[]]->NXM_NX_REG2[[]],resubmit(,8),goto_table:10
+priority=10 arp action=normal
+priority=0,action=drop
+dnl
+dnl MAC resolution table for IP in reg2, stores mac in OXM_OF_PKT_REG0
+table=8,reg2=0x0a0101f0/0xfff0,action=load:0x8088->OXM_OF_PKT_REG0[[]]
+table=8,priority=0,action=load:0->OXM_OF_PKT_REG0[[]]
+dnl ARP responder mac filled in at OXM_OF_PKT_REG0, or 0 for normal action.
+dnl TPA IP in reg2.
+dnl Swaps the fields of the ARP message to turn a query to a response.
+table=10 priority=100 arp xreg0=0 action=normal
+table=10
+priority=10,arp,arp_op=1,action=load:2->OXM_OF_ARP_OP[[]],move:OXM_OF_ARP_SHA[[]]->OXM_OF_ARP_THA[[]],move:OXM_OF_PKT_REG0[[0..47]]->OXM_OF_ARP_SHA[[]],move:OXM_OF_ARP_SPA[[]]->OXM_OF_ARP_TPA[[]],move:NXM_NX_REG2[[]]->OXM_OF_ARP_SPA[[]],move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],move:OXM_OF_PKT_REG0[[0..47]]->NXM_OF_ETH_SRC[[]],move:NXM_OF_IN_PORT[[]]->NXM_NX_REG3[[0..15]],load:0->NXM_OF_IN_PORT[[]],output:NXM_NX_REG3[[0..15]]
+table=10 priority=0 action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl ICMP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [ping -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl
+1 packets transmitted, 1 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e
+'s/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/'], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.2XX,id=,type=0,code=0),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - SNAT with port range with exhaustion])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
-- 
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH]ovs:conntrack: ignore port for icmp/icmpv6 protocol

2019-05-30 Thread solomon


Thank you very much for your suggestions and testcases.
I sent a v2 patch，ref to 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359385.html


Darrell Ball wrote:
> Thanks Solomon; fix looks good.
> 
> Can you:
> 
> 1/ Change subject to:
> 
>  "conntrack: ignore port for ICMP/ICMPv6 NAT."
> 
> 2/ Change commit message body to:
> 
> ICMP/ICMPv6 fails, if the src/dst port is set in a common NAT rule. like
> this:
> actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)
> 
> Fixes: 4cd0481c9e8b ("conntrack: Fix wasted work for ICMP NAT.")
> CC: Darrell Ball 
> Signed-off-by: solomon 
> 
> 3/ Add the following test with the same patch with added
> co-author/signed-off tags
> 
> Signed-off-by: Darrell Ball 
> Co-authored-by: Darrell Ball 
> 
> or if you prefer, I can add the test as a separate patch later without any
> co-authors.
> Either way is fine.
> 
> AT_SETUP([conntrack - SNAT with port range using ICMP])
> CHECK_CONNTRACK()
> CHECK_CONNTRACK_NAT()
> OVS_TRAFFIC_VSWITCHD_START()
> 
> ADD_NAMESPACES(at_ns0, at_ns1)
> 
> ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
> ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> 
> dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from
> ns1->ns0.
> AT_DATA([flows.txt], [dnl
> in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255:2)),2
> in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
> in_port=2,ct_state=+trk,ct_zone=1,action=1
> dnl
> dnl ARP
> priority=100 arp arp_op=1
> action=move:OXM_OF_ARP_TPA[[]]->NXM_NX_REG2[[]],resubmit(,8),goto_table:10
> priority=10 arp action=normal
> priority=0,action=drop
> dnl
> dnl MAC resolution table for IP in reg2, stores mac in OXM_OF_PKT_REG0
> table=8,reg2=0x0a0101f0/0xfff0,action=load:0x8088->OXM_OF_PKT_REG0[[]]
> table=8,priority=0,action=load:0->OXM_OF_PKT_REG0[[]]
> dnl ARP responder mac filled in at OXM_OF_PKT_REG0, or 0 for normal action.
> dnl TPA IP in reg2.
> dnl Swaps the fields of the ARP message to turn a query to a response.
> table=10 priority=100 arp xreg0=0 action=normal
> table=10
> priority=10,arp,arp_op=1,action=load:2->OXM_OF_ARP_OP[[]],move:OXM_OF_ARP_SHA[[]]->OXM_OF_ARP_THA[[]],move:OXM_OF_PKT_REG0[[0..47]]->OXM_OF_ARP_SHA[[]],move:OXM_OF_ARP_SPA[[]]->OXM_OF_ARP_TPA[[]],move:NXM_NX_REG2[[]]->OXM_OF_ARP_SPA[[]],move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],move:OXM_OF_PKT_REG0[[0..47]]->NXM_OF_ETH_SRC[[]],move:NXM_OF_IN_PORT[[]]->NXM_NX_REG3[[0..15]],load:0->NXM_OF_IN_PORT[[]],output:NXM_NX_REG3[[0..15]]
> table=10 priority=0 action=drop
> ])
> 
> AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> 
> dnl ICMP requests from p0->p1 should work fine.
> NS_CHECK_EXEC([at_ns0], [ping -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl
> 1 packets transmitted, 1 received, 0% packet loss, time 0ms
> ])
> 
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e
> 's/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/'], [0], [dnl
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.2XX,id=,type=0,code=0),zone=1
> ])
> 
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
> 
> 
> On Thu, May 30, 2019 at 1:16 AM solomon  wrote:
> 
>> conntrack will not work for icmp/icmpv6 protocol, if the src/dst port is
>> set in nat.
>> like this:
>> actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)
>>
>> This patch fix this. This bug is introduced by commit 4cd0481c9e.
>>
>> commit 4cd0481c9e8b30bca5c0394f4e94ae126bde4908
>> Author: Darrell Ball 
>> Date:   Mon Feb 25 15:36:31 2019 -0800
>>
>> conntrack: Fix wasted work for ICMP NAT.
>>
>>
>> Signed-off-by: solomon 
>> ---
>>  lib/conntrack.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index d7d48a43a..9d6b8a358 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -2039,10 +2039,14 @@ nat_select_range_tuple(struct conntrack *ct, const
>> struct conn *conn,
>>  while (true) {
>>  if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
>>  nat_conn->rev_key.dst.addr = ct_addr;
>> -nat_conn->rev_key.dst.port = htons(port);
>> +if (pat_enabled) {
>> +nat_conn->rev_key.dst.port = htons(port);
>> +}
>>  } else {
>>  nat_conn->rev_key.src.addr = ct_addr;
>> -nat_conn->rev_key.src.port = htons(port);
>> +if (pat_enabled) {
>> +nat_conn->rev_key.src.port = htons(port);
>> +}
>>  }
>>
>>  uint32_t conn_hash = conn_key_hash(_conn->rev_key,
>> --
>> 2.20.1
>>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2]conntrack: ignore port for ICMP/ICMPv6 NAT.

2019-05-30 Thread solomon



ICMP/ICMPv6 fails, if the src/dst port is set in a common NAT rule.
like this:
actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)


Fixes: 4cd0481c9e8b ("conntrack: Fix wasted work for ICMP NAT.")
CC: Darrell Ball 
Signed-off-by: solomon 
Signed-off-by: Darrell Ball 
---
 lib/conntrack.c |  8 +--
 tests/system-traffic.at | 51 +
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index d7d48a43a..9d6b8a358 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2039,10 +2039,14 @@ nat_select_range_tuple(struct conntrack *ct, const 
struct conn *conn,
 while (true) {
 if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
 nat_conn->rev_key.dst.addr = ct_addr;
-nat_conn->rev_key.dst.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.dst.port = htons(port);
+}
 } else {
 nat_conn->rev_key.src.addr = ct_addr;
-nat_conn->rev_key.src.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.src.port = htons(port);
+}
 }
 
 uint32_t conn_hash = conn_key_hash(_conn->rev_key,
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 79d12fecd..7cf7d9392 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3994,6 +3994,57 @@ 
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - SNAT with port range using ICMP])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from
+ns1->ns0.
+AT_DATA([flows.txt], [dnl
+in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240-10.1.1.255:2)),2
+in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
+in_port=2,ct_state=+trk,ct_zone=1,action=1
+dnl
+dnl ARP
+priority=100 arp arp_op=1
+action=move:OXM_OF_ARP_TPA[[]]->NXM_NX_REG2[[]],resubmit(,8),goto_table:10
+priority=10 arp action=normal
+priority=0,action=drop
+dnl
+dnl MAC resolution table for IP in reg2, stores mac in OXM_OF_PKT_REG0
+table=8,reg2=0x0a0101f0/0xfff0,action=load:0x8088->OXM_OF_PKT_REG0[[]]
+table=8,priority=0,action=load:0->OXM_OF_PKT_REG0[[]]
+dnl ARP responder mac filled in at OXM_OF_PKT_REG0, or 0 for normal action.
+dnl TPA IP in reg2.
+dnl Swaps the fields of the ARP message to turn a query to a response.
+table=10 priority=100 arp xreg0=0 action=normal
+table=10
+priority=10,arp,arp_op=1,action=load:2->OXM_OF_ARP_OP[[]],move:OXM_OF_ARP_SHA[[]]->OXM_OF_ARP_THA[[]],move:OXM_OF_PKT_REG0[[0..47]]->OXM_OF_ARP_SHA[[]],move:OXM_OF_ARP_SPA[[]]->OXM_OF_ARP_TPA[[]],move:NXM_NX_REG2[[]]->OXM_OF_ARP_SPA[[]],move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],move:OXM_OF_PKT_REG0[[0..47]]->NXM_OF_ETH_SRC[[]],move:NXM_OF_IN_PORT[[]]->NXM_NX_REG3[[0..15]],load:0->NXM_OF_IN_PORT[[]],output:NXM_NX_REG3[[0..15]]
+table=10 priority=0 action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl ICMP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [ping -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl
+1 packets transmitted, 1 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e
+'s/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/'], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.2XX,id=,type=0,code=0),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - SNAT with port range with exhaustion])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
-- 
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH]ovs:conntrack: ignore port for icmp/icmpv6 protocol

2019-05-30 Thread solomon

conntrack will not work for icmp/icmpv6 protocol, if the src/dst port is set in 
nat.
like this:
actions=ct(nat(dst=172.16.1.100:5000),commit,table=40)

This patch fix this. This bug is introduced by commit 4cd0481c9e.

commit 4cd0481c9e8b30bca5c0394f4e94ae126bde4908
Author: Darrell Ball 
Date:   Mon Feb 25 15:36:31 2019 -0800

conntrack: Fix wasted work for ICMP NAT.


Signed-off-by: solomon 
---
 lib/conntrack.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index d7d48a43a..9d6b8a358 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2039,10 +2039,14 @@ nat_select_range_tuple(struct conntrack *ct, const 
struct conn *conn,
 while (true) {
 if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
 nat_conn->rev_key.dst.addr = ct_addr;
-nat_conn->rev_key.dst.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.dst.port = htons(port);
+}
 } else {
 nat_conn->rev_key.src.addr = ct_addr;
-nat_conn->rev_key.src.port = htons(port);
+if (pat_enabled) {
+nat_conn->rev_key.src.port = htons(port);
+}
 }
 
 uint32_t conn_hash = conn_key_hash(_conn->rev_key,
-- 
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

2019-03-27 Thread solomon

Simon Horman wrote:
 Also need to backport commit 0b4caa2eba to branch2.6~2.10.
>>>
>>> Thanks, I have made preliminary backports to the relevant branches
>>> and am running travisci to see if the tests pass.
>>>
>>> https://travis-ci.org/horms2/ovs/builds/511479533
>>
>> The check above, of the backport to branch-2.10, succeeded so I have
>> pushed the backport to the ovs tree.
>>
>>> https://travis-ci.org/horms2/ovs/builds/511482871
>>> https://travis-ci.org/horms2/ovs/builds/511482977
>>> https://travis-ci.org/horms2/ovs/builds/511482911
>>> https://travis-ci.org/horms2/ovs/builds/511482945
>>
>> The above backports to branches 2.6 to 2.9 were incomplete.
>> I have made a second attempt. Travis is checking over that:
>>
>> https://travis-ci.org/horms2/ovs/builds/511479533
>> https://travis-ci.org/horms2/ovs/builds/511897524
>> https://travis-ci.org/horms2/ovs/builds/511897703
>> https://travis-ci.org/horms2/ovs/builds/511897927
> 
> The above passed and I have now pushed the (complete) backport of
> 0b4caa2eba22 ("ofp-group: support to insert bucket with weight value for
> select type") to branch-2.6 to 2.9.

Thanks for your jobs to backport them.


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

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

2019-03-26 Thread solomon

Ilya Maximets wrote:
>> On Thu, Mar 21, 2019 at 10:41:05AM +0800, solomon wrote:
>>> Ben Pfaff wrote:
>>>> On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:
>>>>>
>>>>> After inserting/removing a bucket, we don't update the bucket counter.
>>>>> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
>>>>
>>>> Thanks for the patch!  It looks correct to me.  Thank you for adding a
>>>> test, too.
>>>>
>>>> I took a closer look and I saw that 'n_buckets' is not very useful,
>>>> because it is only used in cases where the code is already
>>>> O(n_buckets).  I think that we can just remove it.  Then it cannot get
>>>> out-of-sync.  What do you think of this variation of your patch?
>>>
>>>
>>> ovs_list_size() will traversing the list to get the total length.
>>>
>>> In our custom scheduling algorithms (eg wrr, least-connection), 
>>> we need to know the total number of buckets before traversing the bucket 
>>> list to hit target bucket. 
>>> so, it is traversed twice.
>>>
>>> If the number of buckets reaches 100+, there are tens of thousands of 
>>> groups, don't this modification affect performance?
>>>
>>> I hope to keep n_buckets in struct ofgroup.
>>
>> OK.
>>
>> I applied the original to master and backported as far as branch-2.6.
> 
> This patch broke the testsuite on branches 2.6 to 2.10.

The new testcase in this patch insert new bucket using insert-buckets command.
But there is a bug in inserting bucket with weight fixed in commit 
0b4caa2eba22a516a312e7b404107eaebe618ee1
（ofp-group: support to insert bucket with weight value for select type）

Also need to backport commit 0b4caa2eba to branch2.6~2.10.


> 
> Best regards, Ilya Maximets.
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ovs: tests: Adjust the test cases about group stats

2019-03-25 Thread solomon

Roi Dayan wrote> 
> 
> Hi,
> 
> the title of the patch has non utf8 chars after the word 'tests'.
> can you check please.
> 
> I see it as
> 
> ovs: tests???Adjust the test cases about group stats

yes, there is a Chinese colon in original title.

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

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

2019-03-25 Thread solomon


Ben Pfaff wrote:
> On Thu, Mar 21, 2019 at 10:41:05AM +0800, solomon wrote:
>> Ben Pfaff wrote:
>>> On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:
>>>>
>>>> After inserting/removing a bucket, we don't update the bucket counter.
>>>> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
>>>
>>> Thanks for the patch!  It looks correct to me.  Thank you for adding a
>>> test, too.
>>>
>>> I took a closer look and I saw that 'n_buckets' is not very useful,
>>> because it is only used in cases where the code is already
>>> O(n_buckets).  I think that we can just remove it.  Then it cannot get
>>> out-of-sync.  What do you think of this variation of your patch?
>>
>>
>> ovs_list_size() will traversing the list to get the total length.
>>
>> In our custom scheduling algorithms (eg wrr, least-connection), 
>> we need to know the total number of buckets before traversing the bucket 
>> list to hit target bucket. 
>> so, it is traversed twice.
>>
>> If the number of buckets reaches 100+, there are tens of thousands of 
>> groups, don't this modification affect performance?
>>
>> I hope to keep n_buckets in struct ofgroup.
> 
> OK.
> 
> I applied the original to master and backported as far as branch-2.6.

hi Ben，this patch depends on commit 0b4caa2eba22a516a312e7b404107eaebe618ee1
（ofp-group: support to insert bucket with weight value for select type）on 
branch-2.10.

Could you pick commit 0b4caa2eba22 into brnach-2.10 also？

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

[ovs-dev] [PATCH 1/2] ovs: dump bucket id when doing dump-group-stats

2019-03-22 Thread solomon



The number of the buckets that are dumped start from 0 and is incremented.
If the bucket in the group is adjusted, such as delete or added, the order will 
be disrupted.
Then we can't know which bucket the output value corresponds to.

So, also dump the bucket id when doing dump-group-stats command.

Signed-off-by: solomon 
---
 include/openflow/openflow-1.1.h |  4 +++-
 include/openvswitch/ofp-group.h |  8 +++-
 lib/ofp-group.c | 11 ---
 ofproto/ofproto-dpif.c  |  3 ++-
 tests/ofproto.at| 17 +
 5 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index a29db8f3e..91b4dfd1b 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -531,10 +531,12 @@ OFP_ASSERT(sizeof(struct ofp11_group_stats_request) == 8);
 
 /* Used in group stats replies. */
 struct ofp11_bucket_counter {
+ovs_be32 bucket_id;
+uint8_t pad2[4];
 ovs_be64 packet_count;   /* Number of packets processed by bucket. */
 ovs_be64 byte_count; /* Number of bytes processed by bucket. */
 };
-OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 16);
+OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 24);
 
 /* Body of reply to OFPST11_GROUP request */
 struct ofp11_group_stats {
diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h
index cd7af0ebf..db36a2d3e 100644
--- a/include/openvswitch/ofp-group.h
+++ b/include/openvswitch/ofp-group.h
@@ -132,6 +132,12 @@ char *parse_ofp_group_mod_str(struct ofputil_group_mod *, 
int command,
   enum ofputil_protocol *usable_protocols)
 OVS_WARN_UNUSED_RESULT;
 
+struct ofputil_bucket_counter {
+uint32_t bucket_id;
+uint64_t packet_count;   /* Number of packets processed by bucket. */
+uint64_t byte_count; /* Number of bytes processed by bucket. */
+};
+
 /* Group stats reply, independent of protocol. */
 struct ofputil_group_stats {
 uint32_t group_id;/* Group identifier. */
@@ -141,7 +147,7 @@ struct ofputil_group_stats {
 uint32_t duration_sec;  /* UINT32_MAX if unknown. */
 uint32_t duration_nsec;
 uint32_t n_buckets;
-struct bucket_counter *bucket_stats;
+struct ofputil_bucket_counter *bucket_stats;
 };
 
 struct ofpbuf *ofputil_encode_group_stats_request(enum ofp_version,
diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index 6857164f0..42e2f58dd 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -311,6 +311,7 @@ ofputil_group_bucket_counters_to_ofp11(const struct 
ofputil_group_stats *gs,
 int i;
 
 for (i = 0; i < gs->n_buckets; i++) {
+   bucket_cnts[i].bucket_id = htonl(gs->bucket_stats[i].bucket_id);
bucket_cnts[i].packet_count = htonll(gs->bucket_stats[i].packet_count);
bucket_cnts[i].byte_count = htonll(gs->bucket_stats[i].byte_count);
 }
@@ -567,6 +568,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
 
 gs->bucket_stats = xmalloc(gs->n_buckets * sizeof *gs->bucket_stats);
 for (i = 0; i < gs->n_buckets; i++) {
+gs->bucket_stats[i].bucket_id = ntohl(obc[i].bucket_id);
 gs->bucket_stats[i].packet_count = ntohll(obc[i].packet_count);
 gs->bucket_stats[i].byte_count = ntohll(obc[i].byte_count);
 }
@@ -625,9 +627,12 @@ ofputil_group_stats_format(struct ds *s, const struct 
ofp_header *oh)
 
 for (uint32_t bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) {
 if (gs.bucket_stats[bucket_i].packet_count != UINT64_MAX) {
-ds_put_format(s, ",bucket%"PRIu32":", bucket_i);
-ds_put_format(s, "packet_count=%"PRIu64",", 
gs.bucket_stats[bucket_i].packet_count);
-ds_put_format(s, "byte_count=%"PRIu64"", 
gs.bucket_stats[bucket_i].byte_count);
+ds_put_format(s, ",bucket%"PRIu32":",
+gs.bucket_stats[bucket_i].bucket_id);
+ds_put_format(s, "packet_count=%"PRIu64",",
+gs.bucket_stats[bucket_i].packet_count);
+ds_put_format(s, "byte_count=%"PRIu64"",
+gs.bucket_stats[bucket_i].byte_count);
 }
 }
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0da0d0818..f48712e68 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4955,9 +4955,10 @@ group_get_stats(const struct ofgroup *group_, struct 
ofputil_group_stats *ogs)
 ogs->packet_count = group->packet_count;
 ogs->byte_count = group->byte_count;
 
-struct bucket_counter *bucket_stats = ogs->bucket_stats;
+struct ofputil_bucket_counter *bucket_stats = ogs->bucket_stats;
 struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket,

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

2019-03-20 Thread solomon

Ben Pfaff wrote:
> On Wed, Mar 20, 2019 at 08:16:18PM +0800, Li Wei wrote:
>>
>> After inserting/removing a bucket, we don't update the bucket counter.
>> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
> 
> Thanks for the patch!  It looks correct to me.  Thank you for adding a
> test, too.
> 
> I took a closer look and I saw that 'n_buckets' is not very useful,
> because it is only used in cases where the code is already
> O(n_buckets).  I think that we can just remove it.  Then it cannot get
> out-of-sync.  What do you think of this variation of your patch?


ovs_list_size() will traversing the list to get the total length.

In our custom scheduling algorithms (eg wrr, least-connection), 
we need to know the total number of buckets before traversing the bucket list 
to hit target bucket. 
so, it is traversed twice.

If the number of buckets reaches 100+, there are tens of thousands of groups, 
don't this modification affect performance?

I hope to keep n_buckets in struct ofgroup.


> 
> --8<--cut here-->8--
> 
> From: Li Wei 
> Date: Wed, 20 Mar 2019 20:16:18 +0800
> Subject: [PATCH] ofproto: fix the bug of bucket counter is not updated
> 
> After inserting/removing a bucket, we don't update the bucket counter.
> When we call ovs-ofctl dump-group-stats br-int, a panic happened.
> 
> Reproduce steps:
> 1. ovs-ofctl -O OpenFlow15 add-group br-int "group_id=1, type=select, 
> selection_method=hash bucket=bucket_id=1,weight:100,actions=output:1"
> 2. ovs-ofctl insert-buckets br-int "group_id=1, command_bucket_id=last, 
> bucket=bucket_id=7,weight:800,actions=output:1"
> 3. ovs-ofctl dump-group-stats br-int
> 
> gdb) bt
> at ../sysdeps/posix/libc_fatal.c:175
> ar_ptr=) at malloc.c:5049
> group_id=, cb=cb@entry=0x55cab8fd6cd0 ) at 
> ofproto/ofproto.c:6790
> 
> Signed-off-by: solomon 
> Signed-off-by: Ben Pfaff 
> ---
>  ofproto/ofproto-dpif.c |  2 +-
>  ofproto/ofproto-provider.h |  1 -
>  ofproto/ofproto.c  | 11 ---
>  tests/ofproto.at   | 16 
>  4 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 21dd54bab09b..cc6dbc70fe41 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4754,7 +4754,7 @@ static bool
>  group_setup_dp_hash_table(struct group_dpif *group, size_t max_hash)
>  {
>  struct ofputil_bucket *bucket;
> -uint32_t n_buckets = group->up.n_buckets;
> +uint32_t n_buckets = ovs_list_size(>up.buckets);
>  uint64_t total_weight = 0;
>  uint16_t min_weight = UINT16_MAX;
>  struct webster {
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index d1a87a59e47e..a71d911199f4 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -573,7 +573,6 @@ struct ofgroup {
>  const long long int modified; /* Time of last modification. */
>  
>  const struct ovs_list buckets;/* Contains "struct ofputil_bucket"s. 
> */
> -const uint32_t n_buckets;
>  
>  struct ofputil_group_props props;
>  
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 40780e2766ab..ff3b8410bf49 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -6979,11 +6979,12 @@ append_group_stats(struct ofgroup *group, struct 
> ovs_list *replies)
>  long long int now = time_msec();
>  int error;
>  
> -ogs.bucket_stats = xmalloc(group->n_buckets * sizeof *ogs.bucket_stats);
> +size_t n_buckets = ovs_list_size(>buckets);
> +ogs.bucket_stats = xmalloc(n_buckets * sizeof *ogs.bucket_stats);
>  
>  /* Provider sets the packet and byte counts, we do the rest. */
>  ogs.ref_count = rule_collection_n(>rules);
> -ogs.n_buckets = group->n_buckets;
> +ogs.n_buckets = n_buckets;
>  
>  error = (ofproto->ofproto_class->group_get_stats
>   ? ofproto->ofproto_class->group_get_stats(group, )
> @@ -6991,8 +6992,7 @@ append_group_stats(struct ofgroup *group, struct 
> ovs_list *replies)
>  if (error) {
>  ogs.packet_count = UINT64_MAX;
>  ogs.byte_count = UINT64_MAX;
> -memset(ogs.bucket_stats, 0xff,
> -   ogs.n_buckets * sizeof *ogs.bucket_stats);
> +memset(ogs.bucket_stats, 0xff, n_buckets * sizeof *ogs.bucket_stats);
>  }
>  
>  ogs.group_id = group->group_id;
> @@ -7212,9 +7212,6 @@ init_group(struct ofproto *ofproto, const struct 
> ofputil_group_mod *gm,
>   &(*ofgroup)->buckets),
>

Re: [ovs-dev] [crash] trigger a panic when flush-conntrack

2019-03-06 Thread solomon

Darrell Ball wrote:
> +LIST_FOR_EACH_SAFE (conn, next, exp_node, >exp_lists[j]) {
> +if (!zone || *zone == conn->key.zone) {
> +ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);

why do we need this assert?  
Clean the CT_CONN_TYPE_DEFAULT type in conntrack_flush(), and clean 
CT_CONN_TYPE_UN_NAT in nat_clean() like following:
+if ((!zone || *zone == conn->key.zone) &&
+(conn->conn_type == CT_CONN_TYPE_DEFAULT)) {
+//ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);


with the above code, catch an another panic which in ct_clean thread.
I have see the same panic without changeing the code.
Both ct_clean and flush-conntrack, can catch the bad bucket value. 

#0  0x562ae7402553 in hmap_remove (node=0x7f871bdc4258, 
hmap=0x7f9498701c68) at ./include/openvswitch/hmap.h:287
287 while (*bucket != node) {
[Current thread is 1 (Thread 0x7f948700 (LWP 2085796))]
(gdb) bt
#0  0x562ae7402553 in hmap_remove (node=0x7f871bdc4258, 
hmap=0x7f9498701c68) at ./include/openvswitch/hmap.h:287
#1  conn_clean (ct=ct@entry=0x7f9498700d98, conn=0x7f871bdc41b0, 
ctb=ctb@entry=0x7f9498701c38) at lib/conntrack.c:815
#2  0x562ae7402a28 in sweep_bucket (limit=3906, now=1223168469, 
ctb=, ct=0x7f9498700d98) at lib/conntrack.c:1396
#3  conntrack_clean (now=1223168469, ct=0x7f9498700d98) at lib/conntrack.c:1433
#4  clean_thread_main (f_=0x7f9498700d98) at lib/conntrack.c:1488
#5  0x562ae737a48f in ovsthread_wrapper (aux_=) at 
lib/ovs-thread.c:354
#6  0x7f9497715494 in start_thread (arg=0x7f948700) at 
pthread_create.c:333
#7  0x7f9496d09acf in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:97
(gdb) p bucket
$1 = (struct hmap_node **) 0x8  <== why is bucket not a point value?
(gdb) p *(struct hmap *) 0x7f9498701c68
$2 = {buckets = 0x7f8609f8fc00, one = 0x0, mask = 32767, n = 31040}
(gdb) p *(struct hmap_node *) 0x7f871bdc4258
$3 = {hash = 203059667, next = 0x7f8707cfe6c8}
(gdb) p 203059667&32767
$4 = 29139
(gdb) p >buckets[29139]
$5 = (struct hmap_node **) 0x7f8609fc8a98


> +conn_clean(ct, conn, ctb);
> +}
>  }
>  }
> -ct_lock_unlock(>buckets[i].lock);
> +ct_lock_unlock(>lock);
>  }
> 
>  return 0;
> 
> 
> which yields conntrack_flush(...) as
> 
> int
> conntrack_flush(struct conntrack *ct, const uint16_t *zone)
> {
> for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
> struct conntrack_bucket *ctb = >buckets[i];
> ct_lock_lock(>lock);
> for (unsigned j = 0; j < N_CT_TM; j++) {
> struct conn *conn, *next;
> LIST_FOR_EACH_SAFE (conn, next, exp_node, >exp_lists[j]) {
> if (!zone || *zone == conn->key.zone) {
> ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);
> conn_clean(ct, conn, ctb);
> }
>     }
> }
> ct_lock_unlock(>lock);
> }
> 
> return 0;
> }
> 
> Thanks Darrell
> 
> 
> 
> On Wed, Mar 6, 2019 at 8:06 PM solomon  wrote:
> 
>>
>>When i test conntrack, i catch a panic of ovs.
>>
>> Core was generated by `ovs-vswitchd unix:/var/run/openvswitch/db.sock
>> -vconsole:emer -vsyslog:err -vfi'.
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> #0  0x5605c5cd7553 in hmap_remove (node=0x7f734cde0218,
>> hmap=0x7f8178c7fd70) at ./include/openvswitch/hmap.h:287
>> 287 while (*bucket != node) {
>> [Current thread is 1 (Thread 0x7f8178dccb00 (LWP 2024338))]
>> (gdb) bt
>> #0  0x5605c5cd7553 in hmap_remove (node=0x7f734cde0218,
>> hmap=0x7f8178c7fd70) at ./include/openvswitch/hmap.h:287
>> #1  conn_clean (ct=ct@entry=0x7f8178c75d98, conn=0x7f734cde0170,
>> ctb=ctb@entry=0x7f8178c7fd40) at lib/conntrack.c:815
>> #2  0x5605c5cdd66a in conntrack_flush (ct=0x7f8178c75d98, zone=0x0) at
>> lib/conntrack.c:2549
>> #3  0x5605c5bc2b39 in ct_dpif_flush (dpif=0x5605c68a6430,
>> zone=zone@entry=0x0, tuple=tuple@entry=0x0) at lib/ct-dpif.c:140
>> #4  0x5605c5ce17a0 in dpctl_flush_conntrack (argc=argc@entry=1,
>> argv=argv@entry=0x5605c697ec30, dpctl_p=dpctl_p@entry=0x7fffee718110) at
>> lib/dpctl.c:1388
>> #5  0x5605c5cdeb78 in dpctl_unixctl_handler (conn=0x5605c6959ca0,
>> argc=1, argv=0x5605c697ec30, aux=0x5605c5ce1610 ) at
>> lib/dpctl.c:2312
>> #6  0x5605c5c806ea in process_command (request=,
>> conn=0x5605c6959ca0) at lib/unixctl.c:308
>> #7  run_connection (conn=0x5605c6959ca0) at lib/unixctl.c:342
>> #8  unixctl_server_run (server=0x5605c686

[ovs-dev] [crash] trigger a panic when flush-conntrack

2019-03-06 Thread solomon



   When i test conntrack, i catch a panic of ovs.

Core was generated by `ovs-vswitchd unix:/var/run/openvswitch/db.sock 
-vconsole:emer -vsyslog:err -vfi'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x5605c5cd7553 in hmap_remove (node=0x7f734cde0218, 
hmap=0x7f8178c7fd70) at ./include/openvswitch/hmap.h:287
287 while (*bucket != node) {
[Current thread is 1 (Thread 0x7f8178dccb00 (LWP 2024338))]
(gdb) bt
#0  0x5605c5cd7553 in hmap_remove (node=0x7f734cde0218, 
hmap=0x7f8178c7fd70) at ./include/openvswitch/hmap.h:287
#1  conn_clean (ct=ct@entry=0x7f8178c75d98, conn=0x7f734cde0170, 
ctb=ctb@entry=0x7f8178c7fd40) at lib/conntrack.c:815
#2  0x5605c5cdd66a in conntrack_flush (ct=0x7f8178c75d98, zone=0x0) at 
lib/conntrack.c:2549
#3  0x5605c5bc2b39 in ct_dpif_flush (dpif=0x5605c68a6430, 
zone=zone@entry=0x0, tuple=tuple@entry=0x0) at lib/ct-dpif.c:140
#4  0x5605c5ce17a0 in dpctl_flush_conntrack (argc=argc@entry=1, 
argv=argv@entry=0x5605c697ec30, dpctl_p=dpctl_p@entry=0x7fffee718110) at 
lib/dpctl.c:1388
#5  0x5605c5cdeb78 in dpctl_unixctl_handler (conn=0x5605c6959ca0, argc=1, 
argv=0x5605c697ec30, aux=0x5605c5ce1610 ) at 
lib/dpctl.c:2312
#6  0x5605c5c806ea in process_command (request=, 
conn=0x5605c6959ca0) at lib/unixctl.c:308
#7  run_connection (conn=0x5605c6959ca0) at lib/unixctl.c:342
#8  unixctl_server_run (server=0x5605c6868230) at lib/unixctl.c:393
#9  0x5605c5804217 in main (argc=, argv=) at 
vswitchd/ovs-vswitchd.c:126


Environment:
ovs-2.10.1
dpdk-18.0.2.2

How-To-Repeat:
1. configure ovs with snat aciton. 

ovs-ofctl  -O OpenFlow15 add-group $br_name "group_id=1, type=select, 
selection_method=hash 
bucket=bucket_id=1,weight:100,actions=ct(nat(src=172.16.1.1-172.255.255.255),commit,table=40)
  "

2. syn-ddos send tcp syn packet to generate connection tracks.
3. 
#   ovs-appctl dpctl/ct-get-nconns
2063993
#   ovs-appctl dpctl/flush-conntrack

2019-03-07T03:52:24Z|1|unixctl|WARN|error communicating with 
unix:/var/run/openvswitch/ovs-vswitchd.2024338.ctl: End of file
ovs-appctl: ovs-vswitchd: transaction error (End of file)


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

Re: [ovs-dev] [PATCH] ovs: skip ephemeral port for icmp and SNAT in which port range is specified

2019-02-25 Thread solomon

Darrell Ball wrote:
> Thanks for the report
> 
> I think there are two separate issues:
> 1/ Fallback to ephemeral ports for SNAT being less restrictive than in
> kernel
> 2/ Wasted local variable port incrementing work for ICMPv4/v6
> 
> I sent an alternative series here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356654.html
> 

Thanks for your quick ack.
I have seen the accepted patch-set in the upstream.

-- 

Thanks
Solomon

> Darrell
> 
> 
> On Mon, Feb 25, 2019 at 7:03 AM solomon  wrote:
> 
>>
>> If setting the port range in SNAT, we expect that the selected port is in
>> the range we set.
>> At the same time, this behavior is consistent with the kernel-datapath.
>>
>> The port has no meaning for the icmp/icmpv6 protocol.
>> If no key is available, it will exit early.
>>
>> Signed-off-by: LiWei 
>> ---
>>  lib/conntrack.c | 12 +---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 4028ba9a0..c3fd7d63d 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -2115,6 +2115,12 @@ nat_range_hash(const struct conn *conn, uint32_t
>> basis)
>>  return hash_finish(hash, 0);
>>  }
>>
>> +/* This function selects a unique new connection key based on the ip and
>> port
>> + * settings in the nat tuple. For the case where the port range is not
>> + * specified, the ephemeral port from 1024 to 65535 can be selected when
>> the
>> + * address is exhausted.
>> + * However, the ephemeral ports are not enabled by ICMP/ICMPv6 and DNAT.
>> + */
>>  static bool
>>  nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
>> struct conn *nat_conn)
>> @@ -2126,12 +2132,14 @@ nat_select_range_tuple(struct conntrack *ct, const
>> struct conn *conn,
>>  uint16_t max_port;
>>  uint16_t first_port;
>>  uint32_t hash = nat_range_hash(conn, ct->hash_basis);
>> +bool ephemeral_ports_tried = true;
>>
>>  if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
>>  (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
>>  min_port = ntohs(conn->key.src.port);
>>  max_port = ntohs(conn->key.src.port);
>>  first_port = min_port;
>> +ephemeral_ports_tried = false;
>>  } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
>> (!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
>>  min_port = ntohs(conn->key.dst.port);
>> @@ -2175,9 +2183,6 @@ nat_select_range_tuple(struct conntrack *ct, const
>> struct conn *conn,
>>
>>  uint16_t port = first_port;
>>  bool all_ports_tried = false;
>> -/* For DNAT, we don't use ephemeral ports. */
>> -bool ephemeral_ports_tried = conn->nat_info->nat_action &
>> NAT_ACTION_DST
>> - ? true : false;
>>  union ct_addr first_addr = ct_addr;
>>
>>  while (true) {
>> @@ -2190,6 +2195,7 @@ nat_select_range_tuple(struct conntrack *ct, const
>> struct conn *conn,
>>  if ((conn->key.nw_proto == IPPROTO_ICMP) ||
>>  (conn->key.nw_proto == IPPROTO_ICMPV6)) {
>>  all_ports_tried = true;
>> +ephemeral_ports_tried = true;
>>  } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
>>  nat_conn->rev_key.dst.port = htons(port);
>>  } else {
>> --
>> 2.11.0
>>
> 

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

[ovs-dev] [PATCH] ovs: skip ephemeral port for icmp and SNAT in which port range is specified

2019-02-25 Thread solomon



If setting the port range in SNAT, we expect that the selected port is in the 
range we set.
At the same time, this behavior is consistent with the kernel-datapath.

The port has no meaning for the icmp/icmpv6 protocol.
If no key is available, it will exit early.

Signed-off-by: LiWei 
---
 lib/conntrack.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4028ba9a0..c3fd7d63d 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2115,6 +2115,12 @@ nat_range_hash(const struct conn *conn, uint32_t basis)
 return hash_finish(hash, 0);
 }
 
+/* This function selects a unique new connection key based on the ip and port
+ * settings in the nat tuple. For the case where the port range is not
+ * specified, the ephemeral port from 1024 to 65535 can be selected when the
+ * address is exhausted.
+ * However, the ephemeral ports are not enabled by ICMP/ICMPv6 and DNAT.
+ */
 static bool
 nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
struct conn *nat_conn)
@@ -2126,12 +2132,14 @@ nat_select_range_tuple(struct conntrack *ct, const 
struct conn *conn,
 uint16_t max_port;
 uint16_t first_port;
 uint32_t hash = nat_range_hash(conn, ct->hash_basis);
+bool ephemeral_ports_tried = true;
 
 if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
 (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
 min_port = ntohs(conn->key.src.port);
 max_port = ntohs(conn->key.src.port);
 first_port = min_port;
+ephemeral_ports_tried = false;
 } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
(!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
 min_port = ntohs(conn->key.dst.port);
@@ -2175,9 +2183,6 @@ nat_select_range_tuple(struct conntrack *ct, const struct 
conn *conn,
 
 uint16_t port = first_port;
 bool all_ports_tried = false;
-/* For DNAT, we don't use ephemeral ports. */
-bool ephemeral_ports_tried = conn->nat_info->nat_action & NAT_ACTION_DST
- ? true : false;
 union ct_addr first_addr = ct_addr;
 
 while (true) {
@@ -2190,6 +2195,7 @@ nat_select_range_tuple(struct conntrack *ct, const struct 
conn *conn,
 if ((conn->key.nw_proto == IPPROTO_ICMP) ||
 (conn->key.nw_proto == IPPROTO_ICMPV6)) {
 all_ports_tried = true;
+ephemeral_ports_tried = true;
 } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
 nat_conn->rev_key.dst.port = htons(port);
 } else {
-- 
2.11.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 1/1] utilities: Update gdb script so it works with all python versions

2019-01-08 Thread solomon



Eelco Chaudron wrote:
> Newer versions of Python require a different iterator function. This
> change will make the iterator classes work with all Python versions.
> 
> Adds a fix for python3 as it does not support the long() type.
> The fix guaranties the script still works on Python 2.7.
> 
> The uKey walker is rather slow on python3, so added a spinner to
> indicate we are still busy processing entries.
> 
> Fix functions using the iterkeys() function on dictionaries.
> 
> Signed-off-by: Eelco Chaudron 

It's good to me.
Tested-by: solomon 

> ---
> 
> v3: Fixing iterkeys, and adding a spinner
> v2: Adding fix for python3 compatibility as it does not support long()
> 
> utilities/gdb/ovs_gdb.py | 123 ---
>  1 file changed, 103 insertions(+), 20 deletions(-)
> 
> diff --git a/utilities/gdb/ovs_gdb.py b/utilities/gdb/ovs_gdb.py
> index cb9778c69..e8fb44c19 100644
> --- a/utilities/gdb/ovs_gdb.py
> +++ b/utilities/gdb/ovs_gdb.py
> @@ -55,7 +55,11 @@
>  #...
>  #...
>  #
> +from __future__ import print_function
> +
>  import gdb
> +import six
> +import sys
>  import uuid
>  
>  
> @@ -65,6 +69,13 @@ import uuid
>  N_UMAPS = 512
>  
>  
> +#
> +# For Python2-3 compatibility define long as int
> +#
> +if six.PY3:
> +long = int
> +
> +
>  #
>  # The container_of code below is a copied from the Linux kernel project file,
>  # scripts/gdb/linux/utils.py. It has the following copyright header:
> @@ -150,6 +161,37 @@ def eth_addr_to_string(eth_addr):
>  long(eth_addr['ea'][5]))
>  
>  
> +#
> +# Simple class to print a spinner on the console
> +#
> +class ProgressIndicator(object):
> +def __init__(self, message=None):
> +self.spinner = "/-\|"
> +self.spinner_index = 0
> +self.message = message
> +
> +if self.message is not None:
> +print(self.message, end='')
> +
> +def update(self):
> +print("{}\b".format(self.spinner[self.spinner_index]), end='')
> +sys.stdout.flush()
> +self.spinner_index += 1
> +if self.spinner_index >= len(self.spinner):
> +self.spinner_index = 0
> +
> +def pause(self):
> +print("\r\033[K", end='')
> +
> +def resume(self):
> +if self.message is not None:
> +print(self.message, end='')
> +self.update()
> +
> +def done(self):
> +self.pause()
> +
> +
>  #
>  # Class that will provide an iterator over an OVS cmap.
>  #
> @@ -192,7 +234,7 @@ class ForEachCMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  ipml = self.cmap['impl']['p']
>  if ipml['n'] == 0:
>  raise StopIteration
> @@ -206,6 +248,9 @@ class ForEachCMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS hmap.
> @@ -229,7 +274,7 @@ class ForEachHMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  #
>  # In the real implementation the n values is never checked,
>  # however when debugging we do, as we might try to access
> @@ -253,6 +298,9 @@ class ForEachHMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an Netlink attributes
> @@ -268,7 +316,7 @@ class ForEachNL():
>  def round_up(self, val, round_to):
>  return int(val) + (round_to - int(val)) % round_to
>  
> -def next(self):
> +def __next__(self):
>  if self.attr is None or \
> self.attr_len < 4 or self.attr['nla_len'] < 4 or  \
> self.attr['nla_len'] > self.attr_len:
> @@ -286,6 +334,9 @@ class ForEachNL():
>  
>  return attr
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS shash.
> @@ -298,14 +349,17 @@ class ForEachSHASH(ForEachHMAP):
>  super(ForEachSHASH, self).__init__(shash['map'],
> "struct shash_node", "node")
>  
> -def next(self):
> -node = super(ForEachSHASH, self).next()
> +def __next__(self):
&g

Re: [ovs-dev] [PATCH v4] ovs:group: support to insert bucket with weight value for select type

2019-01-06 Thread solomon



Ben Pfaff wrote:
> On Mon, Dec 17, 2018 at 12:24:04PM +0800, solomon wrote:
>> After creating a group with hash select type，then  we need to insert a new 
>> bucket with weight，
>> But，it fails. Commands are as following:
>>
>> # ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
>> selection_method=hash,fields=tcp_src, 
>> bucket=bucket_id=10,weight:99,actions=output:1, 
>> bucket=bucket_id=20,weight:199,actions=output:1 "
>>
>> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
>> command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
>> ovs-ofctl: type is not needed
>>
>> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
>> command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
>> ovs-ofctl: Only select groups can have bucket weights.
>>
>>
>> This patch can help us. However, for other types that are not select, the 
>> check of the parameters
>> is not strict, but it does not affect their function, because other types do 
>> not use this weight parameter.
>>
>>
>> v3--v4:
>> 1. remove the redundant space in testcase。
>>ref：https://api.travis-ci.org/v3/job/467942792/log.txt
>>
>> v2-->v3:
>> 1. add testcase.
>>
>> v1-->v2:
>> 1. fix warning from 0-day robot.
>>
>>
>> Signed-off-by: solomon 
> 
> Thanks for the revision.
> 
> I think that, if we apply this, then every OFPTGC15_INSERT_BUCKET that
> ovs-ofctl sends will include a weight.  That is OK if it is talking to
> the latest ovs-vswitchd, but anything older will reject it.  So, I think
> that it is better if OFPTGC15_INSERT_BUCKET only includes a weight if
> the user supplied one.  I think that we can do that easily, by checking
> for a nonzero weight instead of the command, and if we do that then we
> don't need the command anyway.  So here is what I would suggest folding
> in:
>
I have understood what you mean. And I have send new version patch with v5.

Thanks for your review.


> diff --git a/lib/ofp-group.c b/lib/ofp-group.c
> index d9eba735da58..92ff35842986 100644
> --- a/lib/ofp-group.c
> +++ b/lib/ofp-group.c
> @@ -1177,8 +1177,7 @@ ofputil_put_ofp11_bucket(const struct ofputil_bucket 
> *bucket,
>  static void
>  ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
>   uint32_t bucket_id, enum ofp11_group_type 
> group_type,
> - struct ofpbuf *openflow, enum ofp_version 
> ofp_version,
> - int group_command)
> + struct ofpbuf *openflow, enum ofp_version 
> ofp_version)
>  {
>  struct ofp15_bucket *ob;
>  size_t start, actions_start, actions_len;
> @@ -1190,9 +1189,7 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket 
> *bucket,
>  ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
>   openflow, ofp_version);
>  actions_len = openflow->size - actions_start;
> -
> -if (group_type == OFPGT11_SELECT
> -|| group_command == OFPGC15_INSERT_BUCKET) {
> +if (group_type == OFPGT11_SELECT || bucket->weight) {
>  ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
>  }
>  if (bucket->watch_port != OFPP_ANY) {
> @@ -1269,7 +1266,7 @@ ofputil_append_ofp15_group_desc_reply(const struct 
> ofputil_group_desc *gds,
>  start_buckets = reply->size;
>  LIST_FOR_EACH (bucket, list_node, buckets) {
>  ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
> - gds->type, reply, version, -1);
> + gds->type, reply, version);
>  }
>  ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
>  ogds->type = gds->type;
> @@ -2069,8 +2066,7 @@ ofputil_encode_ofp15_group_mod(enum ofp_version 
> ofp_version,
>  bucket_id = bucket->bucket_id;
>  }
>  
> -ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version,
> - gm->command);
> +ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, 
> ofp_version);
>  }
>  ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
>  ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed 
> < 0
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 0af4fd731573..15a2914836d4 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -679,7 +679,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn insert-bucket

[ovs-dev] [PATCH v5] ovs:group: support to insert bucket with weight value for select type

2019-01-06 Thread solomon

After creating a group with hash select type，then  we need to insert a new 
bucket with weight，
But，it fails. Commands are as following:

# ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
selection_method=hash,fields=tcp_src, 
bucket=bucket_id=10,weight:99,actions=output:1, 
bucket=bucket_id=20,weight:199,actions=output:1 "

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: type is not needed

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: Only select groups can have bucket weights.


This patch can help us. However, for other types that are not select, the check 
of the parameters
is not strict, but it does not affect their function, because other types do 
not use this weight parameter.


v4--v5:
1. As Ben suggested, only set weight with non-zero value in 
OFPTGC15_INSERT_BUCKET command.


v3--v4:
1. remove the redundant space in testcase。
   ref：https://api.travis-ci.org/v3/job/467942792/log.txt

v2-->v3:
1. add testcase.

v1-->v2:
1. fix warning from 0-day robot.


Signed-off-by: solomon 
---
 lib/ofp-group.c  |  8 +---
 tests/ofproto.at | 14 ++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index c9e95ad4c..6857164f0 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1043,7 +1043,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
int command,
 }
 ovs_list_push_back(>buckets, >list_node);
 
-if (gm->type != OFPGT11_SELECT && bucket->weight) {
+if (gm->command != OFPGC15_INSERT_BUCKET
+&& gm->type != OFPGT11_SELECT && bucket->weight) {
 error = xstrdup("Only select groups can have bucket weights.");
 goto out;
 }
@@ -1189,7 +1190,7 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket 
*bucket,
  openflow, ofp_version);
 actions_len = openflow->size - actions_start;
 
-if (group_type == OFPGT11_SELECT) {
+if (group_type == OFPGT11_SELECT || bucket->weight) {
 ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
 }
 if (bucket->watch_port != OFPP_ANY) {
@@ -2251,7 +2252,8 @@ ofputil_check_group_mod(const struct ofputil_group_mod 
*gm)
 
 struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket, list_node, >buckets) {
-if (bucket->weight && gm->type != OFPGT11_SELECT) {
+if (bucket->weight && gm->type != OFPGT11_SELECT
+&& gm->command != OFPGC15_INSERT_BUCKET) {
 return OFPERR_OFPGMFC_INVALID_GROUP;
 }
 
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 6a2cf27c0..15a291483 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -678,6 +678,20 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets 
br0 group_id=1234,comman
 AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn insert-buckets br0 
group_id=1234,command_bucket_id=first,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1],
 [1], [],
   [ovs-ofctl: insert-bucket needs OpenFlow 1.5 or later ('-O OpenFlow15')
 ])
+
+# Verify insert-buckets command to insert bucket with weight value for select 
group.
+AT_CHECK([ovs-ofctl -O OpenFlow15 --strict del-groups br0 group_id=1234])
+AT_DATA([groups.txt], [dnl
+group_id=1234,type=select,selection_method=hash,bucket=bucket_id=1,weight:100,output:11
+])
+AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt])
+AT_CHECK([ovs-ofctl -O OpenFlow15 insert-buckets br0 
group_id=1234,command_bucket_id=last,bucket=bucket_id=2,weight=100,actions=output:11])
+AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
+AT_CHECK([strip_xids < stdout], [0], [dnl
+OFPST_GROUP_DESC reply (OF1.5):
+ 
group_id=1234,type=select,selection_method=hash,bucket=bucket_id:1,weight:100,actions=output:11,bucket=bucket_id:2,weight:100,actions=output:11
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-- 
2.11.0

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

Re: [ovs-dev] [PATCH v2 1/1] utilities: Update gdb script so it works with all python versions

2019-01-06 Thread solomon

Eelco Chaudron wrote:
> Newer versions of Python require a different iterator function. This
> change will make the iterator classes work with all Python versions.
> 
> In addition, it adds a fix for python3 as it does not support the
> long() type. The fix guaranties the script still works on Python
> 2.7.
> 
> Signed-off-by: Eelco Chaudron 

Hi Eeclo:

Test your new patch with all commands, and another two error messages
are reported as following:
(gdb) ovs_dump_netdev
Traceback (most recent call last):
  File "/root/solomon/ovs/utilities/gdb/ovs_gdb.py", line 640, in invoke
self.display_single_netdev(netdev)
  File "/root/solomon/ovs/utilities/gdb/ovs_gdb.py", line 632, in 
display_single_netdev
netdev['auto_classified'], netdev['netdev_class']))
TypeError: unsupported format string passed to gdb.Value.__format__
Error occurred in Python command: unsupported format string passed to 
gdb.Value.__format__
(gdb)

(gdb) ovs_show_fdb
Traceback (most recent call last):
  File "/root/solomon/ovs/utilities/gdb/ovs_gdb.py", line 1106, in invoke
for name in sorted(all_name.iterkeys()):
AttributeError: 'dict' object has no attribute 'iterkeys'
Error occurred in Python command: 'dict' object has no attribute 'iterkeys'
(gdb)



> ---
> 
> v2: Adding fix for python3 compatibility as it does not support long()
> 
>  utilities/gdb/ovs_gdb.py | 49 
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/utilities/gdb/ovs_gdb.py b/utilities/gdb/ovs_gdb.py
> index cb9778c69..74663e577 100644
> --- a/utilities/gdb/ovs_gdb.py
> +++ b/utilities/gdb/ovs_gdb.py
> @@ -56,6 +56,7 @@
>  #...
>  #
>  import gdb
> +import sys
>  import uuid
>  
>  
> @@ -65,6 +66,13 @@ import uuid
>  N_UMAPS = 512
>  
>  
> +#
> +# For Python2-3 compatibility define long as int
> +#
> +if sys.version_info[0] == 3:
> +long = int
> +
> +
>  #
>  # The container_of code below is a copied from the Linux kernel project file,
>  # scripts/gdb/linux/utils.py. It has the following copyright header:
> @@ -192,7 +200,7 @@ class ForEachCMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  ipml = self.cmap['impl']['p']
>  if ipml['n'] == 0:
>  raise StopIteration
> @@ -206,6 +214,9 @@ class ForEachCMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS hmap.
> @@ -229,7 +240,7 @@ class ForEachHMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  #
>  # In the real implementation the n values is never checked,
>  # however when debugging we do, as we might try to access
> @@ -253,6 +264,9 @@ class ForEachHMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an Netlink attributes
> @@ -268,7 +282,7 @@ class ForEachNL():
>  def round_up(self, val, round_to):
>  return int(val) + (round_to - int(val)) % round_to
>  
> -def next(self):
> +def __next__(self):
>  if self.attr is None or \
> self.attr_len < 4 or self.attr['nla_len'] < 4 or  \
> self.attr['nla_len'] > self.attr_len:
> @@ -286,6 +300,9 @@ class ForEachNL():
>  
>  return attr
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS shash.
> @@ -298,14 +315,17 @@ class ForEachSHASH(ForEachHMAP):
>  super(ForEachSHASH, self).__init__(shash['map'],
> "struct shash_node", "node")
>  
> -def next(self):
> -node = super(ForEachSHASH, self).next()
> +def __next__(self):
> +node = super(ForEachSHASH, self).__next__()
>  
>  if self.data_typeobj is None:
>  return node
>  
>  return 
> node['data'].cast(gdb.lookup_type(self.data_typeobj).pointer())
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS simap.
> @@ -315,10 +335,13 @@ class ForEachSIMAP(ForEachHMAP):
>  super(ForEachSIMAP, self).__init__(shash['map'],
>

Re: [ovs-dev] [PATCH 1/1] utilities: Update gdb script so it works with all python versions

2019-01-04 Thread solomon



Eelco Chaudron wrote:
> Newer versions of Python require a different iterator function. This
> change will make the iterator classes work with all Python versions.

Another error is happened when using ovs_dump_udpif_keys command.

The error message is like:
"Error: name 'long' is not defined"

GDB parses python scripts with python3, so need to replace "long" to "int" also.

-- 
Thanks
Solomon


> 
> Signed-off-by: Eelco Chaudron 
> ---
>  utilities/gdb/ovs_gdb.py | 41 ++--
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/utilities/gdb/ovs_gdb.py b/utilities/gdb/ovs_gdb.py
> index cb9778c69..244be2661 100644
> --- a/utilities/gdb/ovs_gdb.py
> +++ b/utilities/gdb/ovs_gdb.py
> @@ -192,7 +192,7 @@ class ForEachCMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  ipml = self.cmap['impl']['p']
>  if ipml['n'] == 0:
>  raise StopIteration
> @@ -206,6 +206,9 @@ class ForEachCMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS hmap.
> @@ -229,7 +232,7 @@ class ForEachHMAP(object):
>  
>  raise StopIteration
>  
> -def next(self):
> +def __next__(self):
>  #
>  # In the real implementation the n values is never checked,
>  # however when debugging we do, as we might try to access
> @@ -253,6 +256,9 @@ class ForEachHMAP(object):
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an Netlink attributes
> @@ -268,7 +274,7 @@ class ForEachNL():
>  def round_up(self, val, round_to):
>  return int(val) + (round_to - int(val)) % round_to
>  
> -def next(self):
> +def __next__(self):
>  if self.attr is None or \
> self.attr_len < 4 or self.attr['nla_len'] < 4 or  \
> self.attr['nla_len'] > self.attr_len:
> @@ -286,6 +292,9 @@ class ForEachNL():
>  
>  return attr
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS shash.
> @@ -298,14 +307,17 @@ class ForEachSHASH(ForEachHMAP):
>  super(ForEachSHASH, self).__init__(shash['map'],
> "struct shash_node", "node")
>  
> -def next(self):
> -node = super(ForEachSHASH, self).next()
> +def __next__(self):
> +node = super(ForEachSHASH, self).__next__()
>  
>  if self.data_typeobj is None:
>  return node
>  
>  return 
> node['data'].cast(gdb.lookup_type(self.data_typeobj).pointer())
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS simap.
> @@ -315,10 +327,13 @@ class ForEachSIMAP(ForEachHMAP):
>  super(ForEachSIMAP, self).__init__(shash['map'],
> "struct simap_node", "node")
>  
> -def next(self):
> -node = super(ForEachSIMAP, self).next()
> +def __next__(self):
> +node = super(ForEachSIMAP, self).__next__()
>  return node['name'], node['data']
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS smap.
> @@ -328,10 +343,13 @@ class ForEachSMAP(ForEachHMAP):
>  super(ForEachSMAP, self).__init__(shash['map'],
>"struct smap_node", "node")
>  
> -def next(self):
> -node = super(ForEachSMAP, self).next()
> +def __next__(self):
> +node = super(ForEachSMAP, self).__next__()
>  return node['key'], node['value']
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Class that will provide an iterator over an OVS list.
> @@ -346,7 +364,7 @@ class ForEachLIST():
>  def __iter__(self):
>  return self
>  
> -def next(self):
> +def __next__(self):
>  if self.list.address == self.node['next']:
>  raise StopIteration
>  
> @@ -359,6 +377,9 @@ class ForEachLIST():
>  gdb.lookup_type(self.typeobj).pointer(),
>  self.member)
>  
> +def next(self):
> +return self.__next__()
> +
>  
>  #
>  # Implements the GDB "ovs_dump_bridges" command
> 


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

Re: [ovs-dev] [PATCH v3] ovs:group: support to insert bucket with weight value for select type

2018-12-17 Thread solomon

Aaron Conole wrote:
> solomon  writes:
> 
>> After creating a group with hash select type，then  we need to insert a new 
>> bucket with weight，
>> But，it fails. Commands are as following:
>>
>> # ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
>> selection_method=hash,fields=tcp_src, 
>> bucket=bucket_id=10,weight:99,actions=output:1, 
>> bucket=bucket_id=20,weight:199,actions=output:1 "
>>
>> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
>> command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
>> ovs-ofctl: type is not needed
>>
>> # ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
>> command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
>> ovs-ofctl: Only select groups can have bucket weights.
>>
>>
>> This patch can help us. However, for other types that are not select, the 
>> check of the parameters
>> is not strict, but it does not affect their function, because other types do 
>> not use this weight parameter.
>>
>>
>> v2-->v3:
>> 1. add testcase.
>>
>> v1-->v2:
>> 1. fix warning from 0-day robot.
>>
>>
>> Signed-off-by: solomon 
>> ---
> 
> Travis build seems to fail with this, and it looks related:
> 
> https://travis-ci.org/ovsrobot/ovs/jobs/467942792
> 
> But, I haven't taken a look at the code (technically, I'm on a frozen
> swamp somewhere in NH not supposed to be looking after work related
> things, but if you don't tell my wife, I won't either :).

hum，I have get the point which cause the fail. Thanks you. 
I will send the new version patch of V4.


../../tests/ofproto.at:690: strip_xids < stdout
--- -   2018-12-14 11:17:41.885100565 +
+++ 
/home/travis/build/ovsrobot/ovs/openvswitch-2.10.90/_build/tests/testsuite.dir/at-groups/946/stdout
 2018-12-14 11:17:41.880292101 +
@@ -1,3 +1,3 @@
 OFPST_GROUP_DESC reply (OF1.5):
- 
group_id=1234,type=select,selection_method=hash,bucket=bucket_id:1,weight:100,actions=output:11,bucket=bucket_id:2,weigh
t:100,actions=output:11
+ 
group_id=1234,type=select,selection_method=hash,bucket=bucket_id:1,weight:100,actions=output:11,bucket=bucket_id:2,weight:100,actions=output:11
 


-- 

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

[ovs-dev] [PATCH v3] ovs:group: support to insert bucket with weight value for select type

2018-12-14 Thread solomon

After creating a group with hash select type，then  we need to insert a new 
bucket with weight，
But，it fails. Commands are as following:

# ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
selection_method=hash,fields=tcp_src, 
bucket=bucket_id=10,weight:99,actions=output:1, 
bucket=bucket_id=20,weight:199,actions=output:1 "

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: type is not needed

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: Only select groups can have bucket weights.


This patch can help us. However, for other types that are not select, the check 
of the parameters
is not strict, but it does not affect their function, because other types do 
not use this weight parameter.


v2-->v3:
1. add testcase.

v1-->v2:
1. fix warning from 0-day robot.


Signed-off-by: solomon 
---
 lib/ofp-group.c  | 17 +++--
 tests/ofproto.at | 14 ++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index c9e95ad4c..d9eba735d 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1043,7 +1043,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
int command,
 }
 ovs_list_push_back(>buckets, >list_node);
 
-if (gm->type != OFPGT11_SELECT && bucket->weight) {
+if (gm->command != OFPGC15_INSERT_BUCKET
+&& gm->type != OFPGT11_SELECT && bucket->weight) {
 error = xstrdup("Only select groups can have bucket weights.");
 goto out;
 }
@@ -1176,7 +1177,8 @@ ofputil_put_ofp11_bucket(const struct ofputil_bucket 
*bucket,
 static void
 ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
  uint32_t bucket_id, enum ofp11_group_type group_type,
- struct ofpbuf *openflow, enum ofp_version ofp_version)
+ struct ofpbuf *openflow, enum ofp_version ofp_version,
+ int group_command)
 {
 struct ofp15_bucket *ob;
 size_t start, actions_start, actions_len;
@@ -1189,7 +1191,8 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket 
*bucket,
  openflow, ofp_version);
 actions_len = openflow->size - actions_start;
 
-if (group_type == OFPGT11_SELECT) {
+if (group_type == OFPGT11_SELECT
+|| group_command == OFPGC15_INSERT_BUCKET) {
 ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
 }
 if (bucket->watch_port != OFPP_ANY) {
@@ -1266,7 +1269,7 @@ ofputil_append_ofp15_group_desc_reply(const struct 
ofputil_group_desc *gds,
 start_buckets = reply->size;
 LIST_FOR_EACH (bucket, list_node, buckets) {
 ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
- gds->type, reply, version);
+ gds->type, reply, version, -1);
 }
 ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
 ogds->type = gds->type;
@@ -2066,7 +2069,8 @@ ofputil_encode_ofp15_group_mod(enum ofp_version 
ofp_version,
 bucket_id = bucket->bucket_id;
 }
 
-ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
+ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version,
+ gm->command);
 }
 ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
 ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
@@ -2251,7 +2255,8 @@ ofputil_check_group_mod(const struct ofputil_group_mod 
*gm)
 
 struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket, list_node, >buckets) {
-if (bucket->weight && gm->type != OFPGT11_SELECT) {
+if (bucket->weight && gm->type != OFPGT11_SELECT
+&& gm->command != OFPGC15_INSERT_BUCKET) {
 return OFPERR_OFPGMFC_INVALID_GROUP;
 }
 
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 9819bc577..7a6598010 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -678,6 +678,20 @@ AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets 
br0 group_id=1234,comman
 AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn insert-buckets br0 
group_id=1234,command_bucket_id=first,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1],
 [1], [],
   [ovs-ofctl: insert-bucket needs OpenFlow 1.5 or later ('-O OpenFlow15')
 ])
+
+#Verify insert-buckets command to insert bucket with weight value for select 
group.
+AT_CHECK([ovs-ofctl -O OpenFlow15 --strict del-groups br0 group_id=1234])
+AT_DATA([groups.txt], [dnl
+group_id=1234,type=select,selection_method=hash,bucket=bucket_id=1,

[ovs-dev] [PATCH v2] ovs:group: support to insert bucket with weight value for select type

2018-11-22 Thread solomon

After creating a group with hash select type，then  we need to insert a new 
bucket with weight，
But，it fails. Commands are as following:

# ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
selection_method=hash,fields=tcp_src, 
bucket=bucket_id=10,weight:99,actions=output:1, 
bucket=bucket_id=20,weight:199,actions=output:1 "

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: type is not needed

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: Only select groups can have bucket weights.


This patch can help us. However, for other types that are not select, the check 
of the parameters
is not strict, but it does not affect their function, because other types do 
not use this weight parameter.

v1-->v2:
1. fix warning from 0-day robot.

Signed-off-by: solomon 
---
 lib/ofp-group.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index c9e95ad4c..6ba15fd82 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1043,7 +1043,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
int command,
 }
 ovs_list_push_back(>buckets, >list_node);
 
-if (gm->type != OFPGT11_SELECT && bucket->weight) {
+if (gm->command != OFPGC15_INSERT_BUCKET
+&& gm->type != OFPGT11_SELECT && bucket->weight) {
 error = xstrdup("Only select groups can have bucket weights.");
 goto out;
 }
@@ -1176,7 +1177,8 @@ ofputil_put_ofp11_bucket(const struct ofputil_bucket 
*bucket,
 static void
 ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
  uint32_t bucket_id, enum ofp11_group_type group_type,
- struct ofpbuf *openflow, enum ofp_version ofp_version)
+ struct ofpbuf *openflow, enum ofp_version ofp_version,
+ int group_command)
 {
 struct ofp15_bucket *ob;
 size_t start, actions_start, actions_len;
@@ -1188,8 +1190,8 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket 
*bucket,
 ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
  openflow, ofp_version);
 actions_len = openflow->size - actions_start;
-
-if (group_type == OFPGT11_SELECT) {
+if (group_type == OFPGT11_SELECT 
+|| group_command == OFPGC15_INSERT_BUCKET) {
 ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
 }
 if (bucket->watch_port != OFPP_ANY) {
@@ -1266,7 +1268,7 @@ ofputil_append_ofp15_group_desc_reply(const struct 
ofputil_group_desc *gds,
 start_buckets = reply->size;
 LIST_FOR_EACH (bucket, list_node, buckets) {
 ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
- gds->type, reply, version);
+ gds->type, reply, version, -1);
 }
 ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
 ogds->type = gds->type;
@@ -2066,7 +2068,8 @@ ofputil_encode_ofp15_group_mod(enum ofp_version 
ofp_version,
 bucket_id = bucket->bucket_id;
 }
 
-ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
+ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version,
+ gm->command);
 }
 ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
 ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
@@ -2251,7 +2254,8 @@ ofputil_check_group_mod(const struct ofputil_group_mod 
*gm)
 
 struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket, list_node, >buckets) {
-if (bucket->weight && gm->type != OFPGT11_SELECT) {
+if (bucket->weight && gm->type != OFPGT11_SELECT &&
+gm->command != OFPGC15_INSERT_BUCKET) {
 return OFPERR_OFPGMFC_INVALID_GROUP;
 }
 
-- 
2.17.2 (Apple Git-113)

-- 

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

[ovs-dev] [PATCH ] ovs:group: support to insert bucket with weight value for select type

2018-11-22 Thread solomon

After creating a group with hash select type，then  we need to insert a new 
bucket with weight，
But，it fails. Commands are as following:

# ovs-ofctl  -O OpenFlow15 add-group br0 "group_id=10, type=select, 
selection_method=hash,fields=tcp_src, 
bucket=bucket_id=10,weight:99,actions=output:1, 
bucket=bucket_id=20,weight:199,actions=output:1 "

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10,type=select 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: type is not needed

# ovs-ofctl -O OpenFlow15 insert-buckets br0 "group_id=10 
command_bucket_id=last,bucket=bucket_id=3,weight=100,actions=output:1"
ovs-ofctl: Only select groups can have bucket weights.


This patch can help us. However, for other types that are not select, the check 
of the parameters
is not strict, but it does not affect their function, because other types do 
not use this weight parameter.

Signed-off-by: LiWei 
---
 lib/ofp-group.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index c9e95ad4c..a4677310a 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1043,7 +1043,7 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
int command,
 }
 ovs_list_push_back(>buckets, >list_node);
 
-if (gm->type != OFPGT11_SELECT && bucket->weight) {
+if (gm->command != OFPGC15_INSERT_BUCKET && gm->type != OFPGT11_SELECT 
&& bucket->weight) {
 error = xstrdup("Only select groups can have bucket weights.");
 goto out;
 }
@@ -1176,7 +1176,8 @@ ofputil_put_ofp11_bucket(const struct ofputil_bucket 
*bucket,
 static void
 ofputil_put_ofp15_bucket(const struct ofputil_bucket *bucket,
  uint32_t bucket_id, enum ofp11_group_type group_type,
- struct ofpbuf *openflow, enum ofp_version ofp_version)
+ struct ofpbuf *openflow, enum ofp_version ofp_version,
+ int group_command)
 {
 struct ofp15_bucket *ob;
 size_t start, actions_start, actions_len;
@@ -1188,8 +1189,7 @@ ofputil_put_ofp15_bucket(const struct ofputil_bucket 
*bucket,
 ofpacts_put_openflow_actions(bucket->ofpacts, bucket->ofpacts_len,
  openflow, ofp_version);
 actions_len = openflow->size - actions_start;
-
-if (group_type == OFPGT11_SELECT) {
+if (group_type == OFPGT11_SELECT || group_command == 
OFPGC15_INSERT_BUCKET) {
 ofpprop_put_u16(openflow, OFPGBPT15_WEIGHT, bucket->weight);
 }
 if (bucket->watch_port != OFPP_ANY) {
@@ -1266,7 +1266,7 @@ ofputil_append_ofp15_group_desc_reply(const struct 
ofputil_group_desc *gds,
 start_buckets = reply->size;
 LIST_FOR_EACH (bucket, list_node, buckets) {
 ofputil_put_ofp15_bucket(bucket, bucket->bucket_id,
- gds->type, reply, version);
+ gds->type, reply, version, -1);
 }
 ogds = ofpbuf_at_assert(reply, start_ogds, sizeof *ogds);
 ogds->type = gds->type;
@@ -2066,7 +2066,7 @@ ofputil_encode_ofp15_group_mod(enum ofp_version 
ofp_version,
 bucket_id = bucket->bucket_id;
 }
 
-ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version);
+ofputil_put_ofp15_bucket(bucket, bucket_id, gm->type, b, ofp_version, 
gm->command);
 }
 ogm = ofpbuf_at_assert(b, start_ogm, sizeof *ogm);
 ogm->command = htons(gm->command != OFPGC11_ADD_OR_MOD || group_existed < 0
@@ -2251,7 +2251,8 @@ ofputil_check_group_mod(const struct ofputil_group_mod 
*gm)
 
 struct ofputil_bucket *bucket;
 LIST_FOR_EACH (bucket, list_node, >buckets) {
-if (bucket->weight && gm->type != OFPGT11_SELECT) {
+if (bucket->weight && gm->type != OFPGT11_SELECT &&
+gm->command != OFPGC15_INSERT_BUCKET) {
 return OFPERR_OFPGMFC_INVALID_GROUP;
 }
 
-- 
2.11.0


-- 

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

[ovs-dev] [PATCH] openvswitch: meter: Remove unnecessary int

[ovs-dev] achat terrains

Re: [ovs-dev] [patch v5] conntrack: ignore port for ICMP/ICMPv6 NAT.

[ovs-dev] [PATCH v3]conntrack: ignore port for ICMP/ICMPv6 NAT.

Re: [ovs-dev] [PATCH]ovs:conntrack: ignore port for icmp/icmpv6 protocol

[ovs-dev] [PATCH v2]conntrack: ignore port for ICMP/ICMPv6 NAT.

[ovs-dev] [PATCH]ovs:conntrack: ignore port for icmp/icmpv6 protocol

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

Re: [ovs-dev] [PATCH 2/2] ovs: tests: Adjust the test cases about group stats

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

[ovs-dev] [PATCH 1/2] ovs: dump bucket id when doing dump-group-stats

Re: [ovs-dev] [PATCH] ovs: fix the bug of bucket counter is not updated

Re: [ovs-dev] [crash] trigger a panic when flush-conntrack

[ovs-dev] [crash] trigger a panic when flush-conntrack

Re: [ovs-dev] [PATCH] ovs: skip ephemeral port for icmp and SNAT in which port range is specified

[ovs-dev] [PATCH] ovs: skip ephemeral port for icmp and SNAT in which port range is specified

Re: [ovs-dev] [PATCH v3 1/1] utilities: Update gdb script so it works with all python versions

Re: [ovs-dev] [PATCH v4] ovs:group: support to insert bucket with weight value for select type

[ovs-dev] [PATCH v5] ovs:group: support to insert bucket with weight value for select type

Re: [ovs-dev] [PATCH v2 1/1] utilities: Update gdb script so it works with all python versions

Re: [ovs-dev] [PATCH 1/1] utilities: Update gdb script so it works with all python versions

Re: [ovs-dev] [PATCH v3] ovs:group: support to insert bucket with weight value for select type

[ovs-dev] [PATCH v3] ovs:group: support to insert bucket with weight value for select type

[ovs-dev] [PATCH v2] ovs:group: support to insert bucket with weight value for select type

[ovs-dev] [PATCH ] ovs:group: support to insert bucket with weight value for select type

26 matches

Site Navigation

Mail list logo

Footer information