On 20/11/2017 14:30, Simon Horman wrote:
On Sun, Nov 19, 2017 at 08:45:19AM +0200, Roi Dayan wrote:
On 16/11/2017 18:29, Simon Horman wrote:
On Wed, Oct 25, 2017 at 02:24:15PM +0300, Roi Dayan wrote:
On 27/09/2017 12:08, Simon Horman wrote:
On Mon, Sep 25, 2017 at 04:31:42PM +0300, Paul Blakey wrote:
On 18/09/2017 18:01, Simon Horman wrote:
On Mon, Sep 18, 2017 at 07:16:03AM +0300, Roi Dayan wrote:
From: Paul Blakey <pa...@mellanox.com>
To be later used to implement ovs action set offloading.
Signed-off-by: Paul Blakey <pa...@mellanox.com>
Reviewed-by: Roi Dayan <r...@mellanox.com>
---
lib/tc.c | 372
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
lib/tc.h | 16 +++
2 files changed, 385 insertions(+), 3 deletions(-)
diff --git a/lib/tc.c b/lib/tc.c
index c9cada2..743b2ee 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -21,8 +21,10 @@
#include <errno.h>
#include <linux/if_ether.h>
#include <linux/rtnetlink.h>
+#include <linux/tc_act/tc_csum.h>
#include <linux/tc_act/tc_gact.h>
#include <linux/tc_act/tc_mirred.h>
+#include <linux/tc_act/tc_pedit.h>
#include <linux/tc_act/tc_tunnel_key.h>
#include <linux/tc_act/tc_vlan.h>
#include <linux/gen_stats.h>
@@ -33,11 +35,14 @@
#include "netlink-socket.h"
#include "netlink.h"
#include "openvswitch/ofpbuf.h"
+#include "openvswitch/util.h"
#include "openvswitch/vlog.h"
#include "packets.h"
#include "timeval.h"
#include "unaligned.h"
+#define MAX_PEDIT_OFFSETS 8
Why 8?
We don't expect anything more right now (ipv6 src/dst rewrite requires 8
pedits iirc). I can't think of a larger use case, maybe ipv6 + macs if
that's makes sens. do you suggest we increase it? to what?
It seems strange to me to place a somewhat arbitrary small limit
when none exists in the pedit API being used. I would at prefer if
it was at least a bigger, say 16 or 32.
Hi Simon,
Sorry for the late reply due to holidays and vacations.
Me & Paul going to go over this and do the fixes needed and
also rebase over latest master and run tests again.
I'll answer what I'm more familiar with now and Paul will continue.
The 8 here is too low and you right. We used this definition
for allocation of the pedit keys on the stack in
nl_msg_put_flower_rewrite_pedits()
It was for convenience instead of calculating the maximum possible
keys that could exists and allocating it there and freeing it at
the end.
Increasing it to 32 is probably more than enough and wont waste much.
I updated the value to 32 when applying the patch.
...
If I understand the above correctly it is designed to make
pedit actions disjoint. If so, why is that necessary? >
It's not, as a single flower key rewrite can be split to multiple pedit
actions it finds the overlap between a flower key and a pedit action, if
they do overlap it translates it to the correct offset and masks it out.
Thanks, understood.
+ } else {
+ VLOG_ERR_RL(&error_rl, "unable to parse legacy pedit type: %d",
+ nl_attr_type(nla));
+ return EOPNOTSUPP;
+ }
I think the code could exit early here as
nl_msg_put_flower_rewrite_pedits() does below.
Sorry, didn't understand. can you give an example?
I meant something like this.
if (nl_attr_type(nla) != TCA_PEDIT_KEY_EX) {
VLOG_ERR_RL(...);
return EOPNOTSUPP;
}
understood. we'll do that. thanks.
I also fixed this when applying the patch.
...
Thanks Simon. sorry we were not responsive enough with this feature.
We'll improve that.
No problem.
Could you work on a fix for the travis failure that Eric Garver flagged?
yes we have a fix. we did more fixes and running some tests. we'll push
the fixes today.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
