[ovs-discuss] table_id becomes zero in PACKET_IN when using write_action(group) instead of group

[Ben Pfaff]

On Wed, Apr 13, 2016 at 05:10:24PM +0000, Zoltán Balogh wrote:
> 
> Hi Ben,
> 
> I found only the definition of ofp_packet_in structure in chapter A.4.1 that mentions table_id.
> 
> /* Packet received on port (datapath -> controller). */
> struct ofp_packet_in {
>     struct ofp_header header;
>     uint32_t buffer_id; /* ID assigned by datapath. */
>     uint16_t total_len; /* Full length of frame. */
>     uint8_t reason; /* Reason packet is being sent (one of OFPR_*) */
>     uint8_t table_id; /* ID of the table that was looked up */
>     uint64_t cookie; /* Cookie of the flow entry that was looked up. */
>     struct ofp_match match; /* Packet metadata. Variable size. */
>     /* Followed by:
>     * - Exactly 2 all-zero padding bytes, then
>     * - An Ethernet frame whose length is inferred from header.length.
>     * The padding bytes preceding the Ethernet frame ensure that the IP
>     * header (if any) following the Ethernet header is 32-bit aligned.
>     */
>     //uint8_t pad[2]; /* Align to 64 bit + 16 bit */
>     //uint8_t data[0]; /* Ethernet frame */
> };
> OFP_ASSERT(sizeof(struct ofp_packet_in) == 32);
> 
> The comment says that table_id member is "the ID of the table that was looked up".
> 
> I investigated the action translator code in ofproto-dpif-xlate.c. I think it would be easy to store the current table ID in a new member of current xlate_ctx structure when action translation through the flow-chain is ongoing and a write_actions(output) or a write_actions(group) action is processed. 
> So this way, the ID of the last table in the flow-chain that contains write_actions(output) or write_actions(group) could be stored. That could be done in xlate_write_actions().
> 
> Then later, when the current action set is processed and there is an output action in the set, then this stored table_id could be used as table_id of current context when translating an output action and output port is CONTROLLER. This should be performed in xlate_output_action().
> 
> What do you think? If it's ok, I would create a patch.

The table lookup is far removed from the output.  The output is caused
by a group, which is in turn caused by the action set, which is in turn
from an action found by a table lookup.  It's not at all obvious to me
that the table lookup is relevant.

What do other OpenFlow switches do?