On 15/08/2017 04:04, Joe Stringer wrote:
On 8 August 2017 at 07:21, Roi Dayan <[email protected]> wrote:
From: Paul Blakey <[email protected]>
Implement support for offloading ovs action set using
tc header rewrite action.
Signed-off-by: Paul Blakey <[email protected]>
Reviewed-by: Roi Dayan <[email protected]>
---
<snip>
@@ -274,6 +346,48 @@ netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
return 0;
}
+static void
+parse_flower_rewrite_to_netlink_action(struct ofpbuf *buf,
+ struct tc_flower *flower)
+{
+ char *mask = (char *) &flower->rewrite.mask;
+ char *data = (char *) &flower->rewrite.key;
+
+ for (int type = 0; type < ARRAY_SIZE(set_flower_map); type++) {
+ char *put = NULL;
+ size_t nested = 0;
+ int len = ovs_flow_key_attr_lens[type].len;
+
+ if (len <= 0) {
+ continue;
+ }
+
+ for (int j = 0; j < ARRAY_SIZE(set_flower_map[type]); j++) {
+ struct netlink_field *f = &set_flower_map[type][j];
+
+ if (!f->size) {
+ break;
+ }
Seems like maybe there's similar feedback here to the previous patch
wrt sparsely populated array set_flower_map[].
I'll answer there.
+
+ if (!is_all_zeros(mask + f->flower_offset, f->size)) {
+ if (!put) {
+ nested = nl_msg_start_nested(buf,
+ OVS_ACTION_ATTR_SET_MASKED);
+ put = nl_msg_put_unspec_zero(buf, type, len * 2);
Do we ever have multiple of these attributes in a single nested
OVS_ACTION_ATTR_SET_MASKED? If so, won't the following memcpys avoid
putting the netlink header for the subsequent sets?
OVS_ACTION_ATTR_SET_MASKED always has a single nested OVS_KEY_ATTR_*
attribute so we put it (all zeros) the first time we find a sub
attribute (e.g ipv4_dst of ipv4) that isn't zero and use memcpy below to
overwrite the non zero sub attributes.
+ }
+
+ memcpy(put + f->offset, data + f->flower_offset, f->size);
+ memcpy(put + len + f->offset,
+ mask + f->flower_offset, f->size);
Could we these combine with the nl_msg_put_unspec() above?
We'd have to accumulate the data as we go over the inner loop and then
write it.
We could, for example, create a stack buffer like char
buff[MAX_ATTR_LEN] (kinda ugly), write it there, and if we wrote
something to buff, flush it to a nested attribute in a single write.
Would that be better? It might be less readable and it is less flexible
(defining of MAX_ATTR_LEN).
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
