[ovs-discuss] [PATCH] ofproto/ofproto-dpif.c don't forward ofp_port == ctx->flow.in_port

Aaron Rosen arosen at clemson.edu
Wed Feb 8 07:16:25 UTC 2012


Fwiw, This is what I think the patch should have been if this is the
desired behavior.

Aaron

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d19b6f7..8903a7f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4545,11 +4545,9 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
     case OFPP_CONTROLLER:
         execute_controller_action(ctx, max_len, OFPR_ACTION);
         break;
-    case OFPP_LOCAL:
-        compose_output_action(ctx, OFPP_LOCAL);
-        break;
     case OFPP_NONE:
         break;
+    case OFPP_LOCAL:
     default:
         if (port != ctx->flow.in_port) {
             compose_output_action(ctx, port);

On Wed, Feb 8, 2012 at 1:44 AM, Ben Pfaff <blp at nicira.com> wrote:
> It's just a bug.
>
> On Tue, Feb 07, 2012 at 10:29:27PM -0800, Ethan Jackson wrote:
>> We already do these checks in ofproto-dpif at the callers of
>> compose_output_action() see the xlate_output_action__() function.  The
>> issue you're running into is that we skip this check for and explicit
>> OFPP_LOCAL output.  I'm not sure if this is an oversight or by design,
>> someone more familiar with the specification will have to chime in.
>>
>> To verify this theory, you can try using the flood action instead
>> which should not flood to the local port.  If it turns out to be an
>> oversight that we aren't performing this check, the appropriate place
>> to fix the behavior would be in xlate_output_action__().
>>
>> Ethan
>>
>> On Tue, Feb 7, 2012 at 17:53, Aaron Rosen <arosen at clemson.edu> wrote:
>> > Or it might be better to just reject this packet?
>> >
>> > Aaron
>> >
>> > On Tue, Feb 7, 2012 at 8:22 PM, Aaron Rosen <arosen at clemson.edu> wrote:
>> >> Hello,
>> >>
>> >> This patch corrects the behavior when an OFPAT_PACKET_OUT is received
>> >> and the in_port is in the output action list. In these cases packets
>> >> should not be sent to the corresponding output if it matches the
>> >> in_port (i.e just like in the case with action=OFPP_FLOOD, the packet
>> >> is not also sent to in_port).
>> >>
>> >> Example:
>> >>  OFPT_PACKET_OUT (xid=0x0): in_port=LOCAL actions_len=16
>> >> actions=output:1,LOCAL data_len=60
>> >>
>> >> This packet is output to ports 1 and LOCAL,but should just output to port 1.
>> >>
>> >> Thanks,
>> >>
>> >> Aaron
>> >>
>> >> Signed-off-by: Aaron Rosen <arosen at clemson.edu>
>> >> ---
>> >>  ofproto/ofproto-dpif.c |    5 +++++
>> >>  1 files changed, 5 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> >> index 28f0434..2fe7a8a 100644
>> >> --- a/ofproto/ofproto-dpif.c
>> >> +++ b/ofproto/ofproto-dpif.c
>> >> @@ -4304,6 +4304,11 @@ compose_output_action__(struct action_xlate_ctx
>> >> *ctx, uint16_t ofp_port,
>> >>     uint8_t flow_nw_tos = ctx->flow.nw_tos;
>> >>     uint16_t out_port;
>> >>
>> >> +    /* If ofp_port equals in_port packet should not be forwarded.
>> >> +     * Only output to OFPP_IN_PORT should output to the in_port. */
>> >> +    if(ofp_port == ctx->flow.in_port) {
>> >> +        return;
>> >> +    }
>> >>     if (ofport) {
>> >>         struct priority_to_dscp *pdscp;
>> >>
>> >> --
>> >> 1.7.3.4
>> >>
>> >>
>> >>
>> >> --
>> >> Aaron O. Rosen
>> >> Masters Student - Network Communication
>> >> 306B Fluor Daniel
>> >
>> >
>> >
>> > --
>> > Aaron O. Rosen
>> > Masters Student - Network Communication
>> > 306B Fluor Daniel
>> > _______________________________________________
>> > discuss mailing list
>> > discuss at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/discuss
>> _______________________________________________
>> discuss mailing list
>> discuss at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/discuss



-- 
Aaron O. Rosen
Masters Student - Network Communication
306B Fluor Daniel



More information about the discuss mailing list