Thanks Jarno, I understand the issue and the need for tracking the set
mask now.
We will stick to the post-xlate cleanup for the time being and possibly
return to introducing a dedicated set-mask at some later stage.
Jan
On 2017-01-12 01:04, Jarno Rajahalme wrote:
On Jan 11, 2017, at 12:10 AM, Jan Scheurich <[email protected]> wrote:
On 2017-01-11 01:14, Jarno Rajahalme wrote:
Unwildcarding of set fields is only done for practical reasons in the userspace
code, and could be optimized away, especially for recent enough datapaths that
implement the masked set action.
Currently, the same bit mask is used to mark what fields have been masked and
what fields have been set. It would be possible to add a separate flow mask for
setting and use the current one only for matching. It will cause some
additional overhead on the upcall processing, but will generate better
megaflows. I remember looking at this when implementing datapath masked set
actions, and vaguely recall someone else also having a go at this later?
Not quite sure I understand why set fields have to be tracked. Can you explain
a bit and point me to the place in the code where the data in wc is used in
this respect?
OVS OpenFlow action processing does not directly execute set actions on a
packet, but on the flow key. Later, in commit_odp_actions(), we translate any
differences between base_flow and flow key into datapath set actions. In
general, datapath set actions act on multiple fields at the same time (e.g.,
Ethernet addresses, IP fields, or L4 ports). Staging the changes made by
OpenFlow actions to the flow key allows coalescing multiple such changes to
less granular datapath actions.
You could look at commit_set_ipv4_action() in lib/odp-util.c for a specific
example.
In the context of L3 tunneling and PTAP the current behavior produces incorrect
results because setting the dl_dst of a packet received from an L3 port adds a
match on dl_dst==00:00:00:00:00:00 to the Megaflow which can never match
because the packet received from the L3 tunnel does not have a dl_dst field.
We might have some code sanitizing the mask that may take care of this, as
there are also other cases where we are setting fields that do not exist in the
incoming packet.
Yes, that's what we have done now in xlate_wc_finish() to clean up. But my gut
feeling is that this shouldn't be needed in the first place. Maybe I'm still
missing some essential piece?
Consider any action that pushes headers to the current packet and then sets any
of the new fields, and/or later stages of the pipeline (maybe through a patch
port) match on those fields. Those fields do not exist in the incoming packet,
and thus need to be excluded from the datapath flow mask. Generally, mask
fields should be specified only for fields that exist in the flow key included
in the upcall.
Jarno
Regards, Jan
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
