On 10/05/2017 16:28, Simon Horman wrote:
On Wed, May 03, 2017 at 06:08:04PM +0300, Roi Dayan wrote:
From: Paul Blakey <[email protected]>
Currently only tunnel offload is supported.
Signed-off-by: Paul Blakey <[email protected]>
Reviewed-by: Roi Dayan <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
lib/dpif-netlink.c | 4 +-
lib/netdev-tc-offloads.c | 392 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 385 insertions(+), 11 deletions(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 81d513d..7e6c647 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1937,8 +1937,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
return EOPNOTSUPP;
}
- memset(&match, 0, sizeof match);
-
err = parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
put->mask_len, &match);
if (err) {
@@ -2011,7 +2009,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
VLOG_DBG("added flow");
} else if (err != EEXIST) {
- VLOG_ERR_RL(&rl, "failed to offload flow: %s", ovs_strerror(err));
+ VLOG_ERR_RL(&rl, "failed adding flow: %s", ovs_strerror(err));
}
out:
I may be misunderstanding things but perhaps the above changes
belong in an earlier patch?
also noticed it and fixed for V9.
the msg was updated in an earlier patch and from rebases the original
string was updated again again here and we missed it.
also for the memset above that was removed. should be in earlier patch.
fixed.
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 7e33fff..e3daf62 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -461,16 +461,392 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
return false;
}
...
+static int
+test_key_and_mask(struct match *match) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+ const struct flow *key = &match->flow;
+ struct flow *mask = &match->wc.masks;
+
+ if (mask->pkt_mark) {
+ VLOG_DBG_RL(&rl, "offloading attribute pkt_mark isn't supported");
+ return EOPNOTSUPP;
+ }
+
+ if (mask->recirc_id && key->recirc_id != 0) {
+ VLOG_DBG_RL(&rl, "offloading attribute recirc_id isn't supported");
+ return EOPNOTSUPP;
+ }
+ mask->recirc_id = 0;
Its not clear to me why the recirc_id mask is reset here.
What I recall is we always get a mask for recirc_id but if the key is 0
we want to ignore the mask and not fail the end test that checked all masks.
+
+ if (mask->dp_hash) {
+ VLOG_DBG_RL(&rl, "offloading attribute dp_hash isn't supported");
+ return EOPNOTSUPP;
+ }
...
+ if (mask->metadata != 0) {
It seems to me that "if (!mask->metdata)" would make the above more
consistent with other checks in this function.
ok
+ VLOG_DBG_RL(&rl, "offloading attribute metadata isn't supported");
+ return EOPNOTSUPP;
+ }
...
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
