Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions
Hi Ben, I have not tried it under OVS sandbox. I can test the patch on both OVS sandbox and my local testbed and update with the results. Thanks & Regards, Anil Kumar -Original Message- From: Ben Pfaff Sent: Friday, 1 November, 2019 11:12 PM To: Ilya Maximets Cc: Ilya Maximets ; Anil Kumar Koli ; ovs-dev Subject: Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions On Fri, Nov 01, 2019 at 02:35:55PM +0100, Ilya Maximets wrote: > CC: ovs-dev > Sorry. > > On 01.11.2019 14:34, Ilya Maximets wrote: > > On 31.10.2019 18:12, Ben Pfaff wrote: > > > On Thu, Sep 05, 2019 at 11:12:26PM +0530, Anil Kumar Koli wrote: > > > > > * We can't get rid of ofproto_mutex in do_bundle_commit(), or > > > > > drop it > > > > > temporarily around flow translation (i.e. the call to > > > > > ofproto_packet_out_start()), because it might need to > > > > > revert all the > > > > > flow table changes and dropping the mutex would allow other > > > > > threads to > > > > > race in and make conflicting changes > > > > > > > > But it seems that the issue happens on > > > > ofproto_packet_out_finish() and not on > > > > ofproto_packet_out_start(). So, the flow translation should be OK under the ofproto_mutex and revert is still possible under the mutex. > > > > The only thing we need to take out of the mutex is real action > > > > execution by the datapath triggered by > > > > ofproto_packet_out_finish(). Callers never check the status of this function so it should be not so hard. > > > > > > > > So, possible solution: > > > > * Move ofproto_packet_out_finish() out of ofproto_mutex in all the callers: > > > > * It's easy for handle_packet_out() > > > > * For do_bundle_commit() we could temporary store all the > > > > ofproto_packet_out > > > > entities and finish them later. > > > > > > > > Am I missing something? Is there reason for > > > > ofproto_packet_out_finish() to require ofproto_mutex? > > > > > > > > Ben, Anil, what do you think? > > > > > > > > Best regards, Ilya Maximets. > > > > > > This does seem like a reasonable solution. Do you want to take a > > > shot at implementing it? I promise to review a patch much more > > > quickly than I responded to the thread :-( > > > > > > > I could try. The hard part here is that I can not reproduce the issue. I can't either. Anil, do you get the crash when you run the five commands cited in your commit message under the OVS sandbox (that you can obtain running "make sandbox")? It would be a lot easier for Ilya and I to work on and talk about this issue if we could reproduce the problem. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 00/10] optimize openvswitch flow looking up
From: xiangxia.m@gmail.com Date: Fri, 1 Nov 2019 22:23:44 +0800 > This series patch optimize openvswitch for performance or simplify > codes. Series applied, thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Investment opportunity
Greetings, Find the attached mail very confidential. reply for more details Thanks. Peter Wong This email was sent by the shareware version of Postman Professional. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v1] vagrant: Use python3 and newer linux distros
Bleep bloop. Greetings Flavio Fernandes, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 84 characters long (recommended limit is 79) #86 FILE: Vagrantfile:35: python-all python-qt4 python-twisted-conch python-zopeinterface \ Lines checked: 181, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] gso packet is failing with af_packet socket with packet_vnet_hdr
Hi,
I am wondering if anyone can help me with this. I am having trouble to send
tso/gso packet
with af_packet socket with packet_vnet_hdr (through virtio_net_hdr) over
vxlan tunnel in OVS.
What I observed that, the following function eventually hitting and is
returning false (net/core/skbuff.c), hence the packet is dropping.
static inline bool skb_gso_size_check(const struct sk_buff *skb,
unsigned int seg_len,
unsigned int max_len) {
const struct skb_shared_info *shinfo = skb_shinfo(skb);
const struct sk_buff *iter;
if (shinfo->gso_size != GSO_BY_FRAGS)
return seg_len <= max_len;
..
}
[ 678.756673] ip_finish_output_gso:235 packet_length:2762 (here
packet_length = skb->len - skb_inner_network_offset(skb))
[ 678.756678] ip_fragment:510 packet length:1500
[ 678.756715] ip_fragment:510 packet length:1314
[ 678.956889] skb_gso_size_check:4474 and seg_len:1550 and max_len:1500
and shinfo->gso_size:1448 and GSO_BY_FRAGS:65535
Observation:
When we send the large packet ( example here is packet_length:2762), its
showing the seg_len(1550) > max_len(1500). Hence return seg_len <= max_len
statement returning false.
Because of this, ip_fragment calling icmp_send(skb, ICMP_DEST_UNREACH,
ICMP_FRAG_NEEDED, htonl(mtu)); rather the code reaching to
ip_finish_output2(sk, skb)
function in net/ipv4/ip_output.c and is given below:
static int ip_finish_output_gso(struct sock *sk, struct sk_buff *skb,
unsigned int mtu)
{
netdev_features_t features;
struct sk_buff *segs;
int ret = 0;
/* common case: seglen is <= mtu */
if (skb_gso_validate_mtu(skb, mtu))
return ip_finish_output2(sk, skb);
...
err = ip_fragment(sk, segs, mtu, ip_finish_output2);
...
}
But when we send normal iperf traffic ( gso/tso traffic) over vxlan, the
skb_gso_size_check returning a true value, and ip_finish_output2 getting
executed.
Here is the values of normal iperf traffic over vxlan.
[ 1041.400537] skb_gso_size_check:4477 and seg_len:1500 and max_len:1500
and shinfo->gso_size:1398 and GSO_BY_FRAGS:65535
[ 1041.400587] skb_gso_size_check:4477 and seg_len:1450 and max_len:1450
and shinfo->gso_size:1398 and GSO_BY_FRAGS:65535
[ 1041.400594] skb_gso_size_check:4477 and seg_len:1500 and max_len:1500
and shinfo->gso_size:1398 and GSO_BY_FRAGS:65535
[ 1041.400732] skb_gso_size_check:4477 and seg_len:1450 and max_len:1450
and shinfo->gso_size:1398 and GSO_BY_FRAGS:65535
[ 1041.400741] skb_gso_size_check:4477 and seg_len:1450 and max_len:1450
and shinfo->gso_size:1398 and GSO_BY_FRAGS:65535
Can someone help me to solve what is missing, and where should I modify the
code in OVS/ or outside of ovs, so that it works as expected.
Thanks in advance.
Some more info:
[root@xx ~]# uname -r
3.10.0-1062.4.1.el7.x86_64
[root@xx ~]# cat /etc/redhat-release
Red Hat Enterprise Linux Server release 7.7 (Maipo)
[root@xx]# ovs-vsctl --version
ovs-vsctl (Open vSwitch) 2.9.0
DB Schema 7.15.1
And dump_stack output with af_packet:
[ 4833.637460][] dump_stack+0x19/0x1b
[ 4833.637474] [] ip_fragment.constprop.55+0xc3/0x141
[ 4833.637481] [] ip_finish_output+0x314/0x350
[ 4833.637484] [] ip_output+0xb3/0x130
[ 4833.637490] [] ? ip_do_fragment+0x910/0x910
[ 4833.637493] [] ip_local_out_sk+0xf9/0x180
[ 4833.637497] [] iptunnel_xmit+0x18c/0x220
[ 4833.637505] [] udp_tunnel_xmit_skb+0x117/0x130
[udp_tunnel]
[ 4833.637538] [] vxlan_xmit_one+0xb6a/0xb70 [vxlan]
[ 4833.637545] [] ? vprintk_default+0x29/0x40
[ 4833.637551] [] vxlan_xmit+0xc9e/0xef0 [vxlan]
[ 4833.637555] [] ? kfree_skbmem+0x37/0x90
[ 4833.637559] [] ? consume_skb+0x34/0x90
[ 4833.637564] [] ? packet_rcv+0x4c/0x3e0
[ 4833.637570] [] dev_hard_start_xmit+0x246/0x3b0
[ 4833.637574] [] __dev_queue_xmit+0x519/0x650
[ 4833.637580] [] ? try_to_wake_up+0x190/0x390
[ 4833.637585] [] dev_queue_xmit+0x10/0x20
[ 4833.637592] [] ovs_vport_send+0xa6/0x180 [openvswitch]
[ 4833.637599] [] do_output+0x4e/0xd0 [openvswitch]
[ 4833.637604] [] do_execute_actions+0xa29/0xa40
[openvswitch]
[ 4833.637610] [] ? __wake_up_common+0x82/0x120
[ 4833.637615] [] ovs_execute_actions+0x4c/0x140
[openvswitch]
[ 4833.637621] [] ovs_dp_process_packet+0x84/0x120
[openvswitch]
[ 4833.637627] [] ? ovs_ct_update_key+0xc4/0x150
[openvswitch]
[ 4833.637633] [] ovs_vport_receive+0x73/0xd0
[openvswitch]
[ 4833.637638] [] ? ttwu_do_activate+0x6f/0x80
[ 4833.637642] [] ? try_to_wake_up+0x190/0x390
[ 4833.637646] [] ? default_wake_function+0x12/0x20
[ 4833.637651] [] ? autoremove_wake_function+0x2b/0x40
[ 4833.637657] [] ? __wake_up_common+0x82/0x120
[ 4833.637661] [] ? update_cfs_shares+0xa9/0xf0
[ 4833.637665] [] ? update_curr+0x86/0x1e0
[ 4833.637669] [] ? __enqueue_entity+0x78/0x80
[ 4833.637677] [] netdev_frame_hook+0xde/0x180
[openvswitch]
[ 4833.637682] []
Re: [ovs-dev] [patch v1] faq: Fix meter action releases.
On Sat, Nov 02, 2019 at 12:05:34PM -0700, Darrell Ball wrote: > At the same time disambiguate some feature descriptions. > 'Meters' is changed to 'Meter action' to clarify that the entry > describes the Openflow meter action rather than port based meters. > 'NAT' is changed to 'Conntrack NAT' to indicate that this entry > represents NAT done in 'conntrack', rather than basic Openflow > IP address and L4 port modifications. > > Signed-off-by: Darrell Ball Thanks a lot for looking at this. Probably no one else would have noticed these discrepancies. I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v1] vagrant: Use python3 and newer linux distros
Stop usage of python2 as ovs+ovn no longer support it. Update vagrant boxes to the following revisions: - Debian: buster (from jessie) - Fedora: v31 (from v29) - Centos: 8 (from 7, kinda**) Centos 7 may still be used, but only if explicitly provided in command: 'vagrant up centos-7' Fedora-31's dnf is not reliable. This patch uses a max retry loop to improve the odds of success. Not all tests are passing when doing 'make check' and that is not related to this change [1]. While provisioning will invoke 'make check', failures will be ignored based on this variable: exit_rc_when_failed. The provisioning does not yet include building rpms+deb packages. [1]: https://github.com/ovn-org/ovn/issues/23 Signed-off-by: Flavio Fernandes --- Vagrantfile | 96 + 1 file changed, 68 insertions(+), 28 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 07ed0b0e0..88c981c71 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -6,41 +6,58 @@ VAGRANTFILE_API_VERSION = "2" Vagrant.require_version ">=1.7.0" $bootstrap_ovs_fedora =
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Prevent duplicating of traffic to a mirror port
On 2019-11-03 12:00 PM, 0-day Robot wrote: > Bleep bloop. Greetings Roi Dayan, I am a robot and I have tried out your > patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > checkpatch: > WARNING: Line is 83 characters long (recommended limit is 79) > #31 FILE: ofproto/ofproto-dpif-xlate.c:3125: > xlate_report_error(ctx, "dropping packet received on port > %s, " > > WARNING: Line is 85 characters long (recommended limit is 79) > #32 FILE: ofproto/ofproto-dpif-xlate.c:3126: >"which is reserved exclusively for > mirroring", > > Lines checked: 45, Warnings: 2, Errors: 0 > It's the same prints taken from little up in the function when the same drop happens. so to be consistent it was left the same. > > Please check this out. If you feel there has been an error, please email > acon...@redhat.com > > Thanks, > 0-day Robot > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Prevent duplicating of traffic to a mirror port
Bleep bloop. Greetings Roi Dayan, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 83 characters long (recommended limit is 79) #31 FILE: ofproto/ofproto-dpif-xlate.c:3125: xlate_report_error(ctx, "dropping packet received on port %s, " WARNING: Line is 85 characters long (recommended limit is 79) #32 FILE: ofproto/ofproto-dpif-xlate.c:3126: "which is reserved exclusively for mirroring", Lines checked: 45, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif-xlate: Prevent duplicating of traffic to a mirror port
From: Dmytro Linkin
Currently ofproto design disallow duplicating output packet on forwarding
and mirroring to/from same ovs port. Next scenario reveal lack of design:
1. Send ping between regular ovs ports (VFs, for ex.), stop it.
2. While rule still exist, make mirror for one of the ports.
Prevent duplicating of traffic to a mirror port.
Fixes: 86e2dcddce85 ("dpif-xlate: Snoop multicast packets and send them
properly")
Signed-off-by: Dmytro Linkin
Acked-by: Roi Dayan
---
ofproto/ofproto-dpif-xlate.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f92cb62c80ce..935a44dd07c2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3118,6 +3118,19 @@ xlate_normal(struct xlate_ctx *ctx)
if (mac_port) {
struct xbundle *mac_xbundle = xbundle_lookup(ctx->xcfg, mac_port);
+
+/* Drop frames if output port is a mirror port. */
+if (mac_xbundle && xbundle_mirror_out(ctx->xbridge, mac_xbundle)) {
+if (ctx->xin->packet != NULL) {
+xlate_report_error(ctx, "dropping packet received on port
%s, "
+ "which is reserved exclusively for
mirroring",
+ mac_xbundle->name);
+}
+xlate_report(ctx, OFT_WARN,
+ "output port is a mirror port, dropping");
+return;
+}
+
if (mac_xbundle
&& mac_xbundle != in_xbundle
&& mac_xbundle->ofbundle != in_xbundle->ofbundle) {
--
2.8.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] lib/tc: Fix flow dump for tunnel id equal zero
On 2019-11-02 1:27 PM, Simon Horman wrote:
> On Fri, Nov 01, 2019 at 03:54:49PM +0100, Simon Horman wrote:
>> On Wed, Oct 30, 2019 at 02:40:35PM +0200, Roi Dayan wrote:
>>> From: Dmytro Linkin
>>>
>>> Tunnel id 0 is not printed unless tunnel flag FLOW_TNL_F_KEY is set.
>>> Fix that by always setting FLOW_TNL_F_KEY when tunnel id is valid.
>>>
>>> Fixes: 0227bf092ee6 ("lib/tc: Support optional tunnel id")
>>> Signed-off-by: Dmytro Linkin
>>> Reviewed-by: Roi Dayan
>>
>> Hi Roi,
>>
>> this looks fine to me but I am holding off on pushing it
>> until master passes travis-ci again.
>>
>> It also seems to backport cleanly to branch-2.11 and I plan
>
> s/11/12/
>
>> to apply it there too.
>
> I have now pushed this patch to master and branch-2.12.
>
>>
>> If it is suitable for older branches could you please post a
>> backport/backports?
The fix is not relevant for other branches as the original commit
to support tunnel id 0 is also not in other branches.
We treated this as a feature and didn't add to old branches.
Thanks
>>
>> Thanks
>>
>>> ---
>>> lib/netdev-offload-tc.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>>> index f6d1abb2e695..502e73ad5332 100644
>>> --- a/lib/netdev-offload-tc.c
>>> +++ b/lib/netdev-offload-tc.c
>>> @@ -600,6 +600,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
>>> if (flower->tunnel) {
>>> if (flower->mask.tunnel.id) {
>>> match_set_tun_id(match, flower->key.tunnel.id);
>>> +match->flow.tunnel.flags |= FLOW_TNL_F_KEY;
>>> }
>>> if (flower->key.tunnel.ipv4.ipv4_dst) {
>>> match_set_tun_src(match, flower->key.tunnel.ipv4.ipv4_src);
>>> --
>>> 2.8.4
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Croid%40mellanox.com%7C1816ea5da15e484648cd08d75f879fc6%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637082908414787875sdata=yx%2F%2Braivnd76D6ydg0mGsEEW%2BMiV6uIGawTi%2FYlyciA%3Dreserved=0
>>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Croid%40mellanox.com%7C1816ea5da15e484648cd08d75f879fc6%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637082908414787875sdata=yx%2F%2Braivnd76D6ydg0mGsEEW%2BMiV6uIGawTi%2FYlyciA%3Dreserved=0
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 10/10] net: openvswitch: simplify the ovs_dp_cmd_new
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > use the specified functions to init resource. > > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 09/10] net: openvswitch: don't unlock mutex when changing the user_features fails
On Fri, Nov 1, 2019 at 7:24 AM wrote:
>
> From: Tonghao Zhang
>
> Unlocking of a not locked mutex is not allowed.
> Other kernel thread may be in critical section while
> we unlock it because of setting user_feature fail.
>
> Fixes: 95a7233c4 ("net: openvswitch: Set OvS recirc_id from tc chain index")
> Cc: Paul Blakey
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> Acked-by: William Tu
> ---
Acked-by: Pravin B Shelar
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 08/10] net: openvswitch: fix possible memleak on destroy flow-table
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > When we destroy the flow tables which may contain the flow_mask, > so release the flow mask struct. > > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > --- Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 07/10] net: openvswitch: add likely in flow_lookup
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > The most case *index < ma->max, and flow-mask is not NULL. > We add un/likely for performance. > > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > Acked-by: William Tu > --- Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 05/10] net: openvswitch: optimize flow-mask looking up
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > The full looking up on flow table traverses all mask array. > If mask-array is too large, the number of invalid flow-mask > increase, performance will be drop. > > One bad case, for example: M means flow-mask is valid and NULL > of flow-mask means deleted. > > +---+ > | M | NULL | ... | NULL | M| > +---+ > > In that case, without this patch, openvswitch will traverses all > mask array, because there will be one flow-mask in the tail. This > patch changes the way of flow-mask inserting and deleting, and the > mask array will be keep as below: there is not a NULL hole. In the > fast path, we can "break" "for" (not "continue") in flow_lookup > when we get a NULL flow-mask. > > "break" > v > +---+ > | M | M | NULL |... | NULL | NULL| > +---+ > > This patch don't optimize slow or control path, still using ma->max > to traverse. Slow path: > * tbl_mask_array_realloc > * ovs_flow_tbl_lookup_exact > * flow_mask_find > > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > --- Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 06/10] net: openvswitch: simplify the flow_hash
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > Simplify the code and remove the unnecessary BUILD_BUG_ON. > > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > Acked-by: William Tu > --- Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 04/10] net: openvswitch: optimize flow mask cache hash collision
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > Port the codes to linux upstream and with little changes. > > Pravin B Shelar, says: > | In case hash collision on mask cache, OVS does extra flow > | lookup. Following patch avoid it. > > Link: > https://github.com/openvswitch/ovs/commit/0e6efbe2712da03522532dc5e84806a96f6a0dd1 > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > --- Signed-off-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 03/10] net: openvswitch: shrink the mask array if necessary
On Fri, Nov 1, 2019 at 7:24 AM wrote:
>
> From: Tonghao Zhang
>
> When creating and inserting flow-mask, if there is no available
> flow-mask, we realloc the mask array. When removing flow-mask,
> if necessary, we shrink mask array.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> Acked-by: William Tu
> ---
Acked-by: Pravin B Shelar
> net/openvswitch/flow_table.c | 33 +++--
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 92efa23..0c0fcd6 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -694,6 +694,23 @@ static struct table_instance
> *table_instance_expand(struct table_instance *ti,
> return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
> }
>
> +static void tbl_mask_array_delete_mask(struct mask_array *ma,
> + struct sw_flow_mask *mask)
> +{
> + int i;
> +
> + /* Remove the deleted mask pointers from the array */
> + for (i = 0; i < ma->max; i++) {
> + if (mask == ovsl_dereference(ma->masks[i])) {
> + RCU_INIT_POINTER(ma->masks[i], NULL);
> + ma->count--;
> + kfree_rcu(mask, rcu);
> + return;
> + }
> + }
> + BUG();
> +}
> +
> /* Remove 'mask' from the mask list, if it is not needed any more. */
> static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask
> *mask)
> {
> @@ -707,18 +724,14 @@ static void flow_mask_remove(struct flow_table *tbl,
> struct sw_flow_mask *mask)
>
> if (!mask->ref_count) {
> struct mask_array *ma;
> - int i;
>
> ma = ovsl_dereference(tbl->mask_array);
> - for (i = 0; i < ma->max; i++) {
> - if (mask == ovsl_dereference(ma->masks[i])) {
> - RCU_INIT_POINTER(ma->masks[i], NULL);
> - ma->count--;
> - kfree_rcu(mask, rcu);
> - return;
> - }
> - }
> - BUG();
> + tbl_mask_array_delete_mask(ma, mask);
> +
> + /* Shrink the mask array if necessary. */
> + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) &&
> + ma->count <= (ma->max / 3))
> + tbl_mask_array_realloc(tbl, ma->max / 2);
> }
> }
> }
> --
> 1.8.3.1
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 02/10] net: openvswitch: convert mask list in mask array
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > Port the codes to linux upstream and with little changes. > > Pravin B Shelar, says: > | mask caches index of mask in mask_list. On packet recv OVS > | need to traverse mask-list to get cached mask. Therefore array > | is better for retrieving cached mask. This also allows better > | cache replacement algorithm by directly checking mask's existence. > > Link: > https://github.com/openvswitch/ovs/commit/d49fc3ff53c65e4eca9cabd52ac63396746a7ef5 > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > Acked-by: William Tu > --- Signed-off-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v6 01/10] net: openvswitch: add flow-mask cache for performance
On Fri, Nov 1, 2019 at 7:24 AM wrote: > > From: Tonghao Zhang > > The idea of this optimization comes from a patch which > is committed in 2014, openvswitch community. The author > is Pravin B Shelar. In order to get high performance, I > implement it again. Later patches will use it. > > Pravin B Shelar, says: > | On every packet OVS needs to lookup flow-table with every > | mask until it finds a match. The packet flow-key is first > | masked with mask in the list and then the masked key is > | looked up in flow-table. Therefore number of masks can > | affect packet processing performance. > > Link: > https://github.com/openvswitch/ovs/commit/5604935e4e1cbc16611d2d97f50b717aa31e8ec5 > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > Acked-by: William Tu > --- Signed-off-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
