> On Mar 25, 2015, at 3:47 PM, Ethan Jackson <[email protected]> wrote:
>
> Copyright year in ofproto-dpif-rid.c needs to be updated.
>
Done.
> In compose_output_action__() it's not clear to me why we don't pass
> the table_id to xlate_table_action() anymore.
>
Earlier, before recirculation IDs were associated with an ofproto, we used to
execute the output action to a patch port again, if the peer recirculated. It
is no longer the case, so we can always start translation from table 0 when we
output to a patch port. In this patch I removed the function we used to set the
‘table_id’ to 0 or TBL_INTERNAL ("uint8_t table_id =
rule_dpif_lookup_get_init_table_id(&ctx->xin->flow);”) so it made sense to
always pass a zero instead.
> Ask discussed offline, I think it would be a bit cleaner to garbage
> collect the recird ids. That said, I don't think it should block the
> patch.
>
I’ll look in to this some time later.
> Also itd be nice to switch bonds to using this same framework.
>
They already partially do, but the record ID is “owned” by the internal rule.
Doing this is also on my TODO list :-)
> Acked-by: Ethan Jackson <[email protected]>
>
Thanks for the review!
Pushed to master,
Jarno
>
> On Tue, Mar 24, 2015 at 11:08 AM, Jarno Rajahalme <[email protected]>
> wrote:
>> Thank you for the review. Ethan promised to review this as well, so I’ll
>> wait for his verdict before merging this.
>>
>> Jarno
>>
>>> On Mar 24, 2015, at 9:56 AM, Ben Pfaff <[email protected]> wrote:
>>>
>>> On Thu, Mar 19, 2015 at 06:03:28PM -0700, Jarno Rajahalme wrote:
>>>> xlate_actions() now considers an optional recirculation context (via
>>>> 'xin') and restores OpenFlow pipeline metadata (registers, 'metadata',
>>>> etc.) based on it. The recirculation context may contain an action
>>>> set and stack to be restored and further actions to be executed upon
>>>> recirculation. It also contains a table_id number to be used for rule
>>>> lookup in cases where no post-recirculation actions are used.
>>>>
>>>> The translation context internal metadata is restored using a new
>>>> internal action: UNROLL_XLATE action stores the translation context
>>>> data visible to OpenFlow controllers via PACKET_IN messages. This
>>>> includes the current table number and the current rule cookie.
>>>> UNROLL_XLATE actions are inserted only when the remaining actions may
>>>> generate PACKET_IN messages.
>>>>
>>>> These changes allow the post-MPLS recirculation to properly continue
>>>> with the pipeline metadata that existed at the time of recirculation.
>>>>
>>>> The internal table is still consulted for bonds.
>>>>
>>>> Signed-off-by: Jarno Rajahalme <[email protected]>
>>>
>>> I've read this a few times before and I feel like I can't properly
>>> re-review it. I did a quick scroll-through this time and nothing jumped
>>> out. If you want a thorough review, you should ask someone with fresh
>>> eyes, but short of that:
>>>
>>> Acked-by: Ben Pfaff <[email protected]>
>>>
>>> Thanks so much for persisting with this big job!
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
