Thanks.  I'll fold that in for the next version.

On Mon, Jun 19, 2017 at 03:53:31PM +0000, Zoltán Balogh wrote:
> Hi Ben,
> 
> The bridge property 'other-config:legacy-l3-pipeline' is obsolete, so the 3 
> lines starting at line 4197 in ofproto.at can be removed.
> 
> > +AT_CHECK([
> > +    ovs-vsctl set bridge br0 other-config:legacy-l3-pipeline=false
> > +], [0])
> 
> 
> Best regards,
> Zoltan
> 
> > -----Original Message-----
> > From: Ben Pfaff [mailto:[email protected]]
> > Sent: Monday, June 19, 2017 1:30 AM
> > To: [email protected]
> > Cc: Ben Pfaff <[email protected]>; Zoltán Balogh <[email protected]>; 
> > Jean Tourrilhes <[email protected]>; Jan
> > Scheurich <[email protected]>
> > Subject: [PATCH v2 11/12] userspace: Introduce packet_type in OF 1.5 
> > packet-out
> > 
> > From: Zoltán Balogh <[email protected]>
> > 
> > Introducing packet_type in OF 1.5 packet-out.
> > Partly based on Jean Tourrilhes's work.
> > 
> > Add test cases for OF1.5 packet-out
> > Add negative test case for OF1.5 packet-out
> > Modify wildcarding and packet-out test printout.
> > 
> > Signed-off-by: Jean Tourrilhes <[email protected]>
> > Signed-off-by: Zoltan Balogh <[email protected]>
> > Co-authored-by: Jan Scheurich <[email protected]>
> > Signed-off-by: Ben Pfaff <[email protected]>
> > ---
> >  lib/flow.c                                  | 36 +++++++-----
> >  lib/ofp-parse.c                             | 13 +++++
> >  lib/ofp-print.c                             |  4 +-
> >  lib/ofp-util.c                              |  2 +
> >  ofproto/ofproto.c                           |  3 +
> >  tests/ofproto.at                            | 85 
> > +++++++++++++++++++++++++++++
> >  tests/system-userspace-packet-type-aware.at |  2 +-
> >  utilities/ovs-ofctl.c                       |  1 +
> >  8 files changed, 128 insertions(+), 18 deletions(-)
> > 
> > diff --git a/lib/flow.c b/lib/flow.c
> > index dbca4d03da3d..75a91cc6a2f3 100644
> > --- a/lib/flow.c
> > +++ b/lib/flow.c
> > @@ -1441,6 +1441,8 @@ void
> >  flow_wildcards_init_for_packet(struct flow_wildcards *wc,
> >                                 const struct flow *flow)
> >  {
> > +    ovs_be16 dl_type = OVS_BE16_MAX;
> > +
> >      memset(&wc->masks, 0x0, sizeof wc->masks);
> > 
> >      /* Update this function whenever struct flow changes. */
> > @@ -1493,25 +1495,29 @@ flow_wildcards_init_for_packet(struct 
> > flow_wildcards *wc,
> >      /* actset_output wildcarded. */
> > 
> >      WC_MASK_FIELD(wc, packet_type);
> > -    WC_MASK_FIELD(wc, dl_dst);
> > -    WC_MASK_FIELD(wc, dl_src);
> > -    WC_MASK_FIELD(wc, dl_type);
> > -
> > -    /* No need to set mask of inner VLANs that don't exist. */
> > -    for (int i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
> > -        /* Always show the first zero VLAN. */
> > -        WC_MASK_FIELD(wc, vlans[i]);
> > -        if (flow->vlans[i].tci == htons(0)) {
> > -            break;
> > +    if (flow->packet_type == htonl(PT_ETH)) {
> > +        WC_MASK_FIELD(wc, dl_dst);
> > +        WC_MASK_FIELD(wc, dl_src);
> > +        WC_MASK_FIELD(wc, dl_type);
> > +        /* No need to set mask of inner VLANs that don't exist. */
> > +        for (int i = 0; i < FLOW_MAX_VLAN_HEADERS; i++) {
> > +            /* Always show the first zero VLAN. */
> > +            WC_MASK_FIELD(wc, vlans[i]);
> > +            if (flow->vlans[i].tci == htons(0)) {
> > +                break;
> > +            }
> >          }
> > +        dl_type = flow->dl_type;
> > +    } else {
> > +        dl_type = pt_ns_type_be(flow->packet_type);
> >      }
> > 
> > -    if (flow->dl_type == htons(ETH_TYPE_IP)) {
> > +    if (dl_type == htons(ETH_TYPE_IP)) {
> >          WC_MASK_FIELD(wc, nw_src);
> >          WC_MASK_FIELD(wc, nw_dst);
> >          WC_MASK_FIELD(wc, ct_nw_src);
> >          WC_MASK_FIELD(wc, ct_nw_dst);
> > -    } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> > +    } else if (dl_type == htons(ETH_TYPE_IPV6)) {
> >          WC_MASK_FIELD(wc, ipv6_src);
> >          WC_MASK_FIELD(wc, ipv6_dst);
> >          WC_MASK_FIELD(wc, ipv6_label);
> > @@ -1523,15 +1529,15 @@ flow_wildcards_init_for_packet(struct 
> > flow_wildcards *wc,
> >              WC_MASK_FIELD(wc, ct_ipv6_src);
> >              WC_MASK_FIELD(wc, ct_ipv6_dst);
> >          }
> > -    } else if (flow->dl_type == htons(ETH_TYPE_ARP) ||
> > -               flow->dl_type == htons(ETH_TYPE_RARP)) {
> > +    } else if (dl_type == htons(ETH_TYPE_ARP) ||
> > +               dl_type == htons(ETH_TYPE_RARP)) {
> >          WC_MASK_FIELD(wc, nw_src);
> >          WC_MASK_FIELD(wc, nw_dst);
> >          WC_MASK_FIELD(wc, nw_proto);
> >          WC_MASK_FIELD(wc, arp_sha);
> >          WC_MASK_FIELD(wc, arp_tha);
> >          return;
> > -    } else if (eth_type_mpls(flow->dl_type)) {
> > +    } else if (eth_type_mpls(dl_type)) {
> >          for (int i = 0; i < FLOW_MAX_MPLS_LABELS; i++) {
> >              WC_MASK_FIELD(wc, mpls_lse[i]);
> >              if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) {
> > diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> > index 8e2448b20dbd..528b75b4f4e1 100644
> > --- a/lib/ofp-parse.c
> > +++ b/lib/ofp-parse.c
> > @@ -667,6 +667,19 @@ parse_ofp_packet_out_str__(struct ofputil_packet_out 
> > *po, char *string,
> >                  goto out;
> >              }
> >              match_set_in_port(&po->flow_metadata, in_port);
> > +        } else if (!strcmp(name, "packet_type")) {
> > +            char *ns = value;
> > +            char *ns_type = strstr(value, ",");
> > +            if (ns_type) {
> > +                ovs_be32 packet_type;
> > +                *ns_type = '\0';
> > +                packet_type = PACKET_TYPE_BE(strtoul(ns, NULL, 0),
> > +                                             strtoul(++ns_type, NULL, 0));
> > +                match_set_packet_type(&po->flow_metadata, packet_type);
> > +            } else {
> > +                error = xasprintf("%s(%s) can't be interpreted", name, 
> > value);
> > +                goto out;
> > +            }
> >          } else if (!strcmp(name, "packet")) {
> >              const char *error_msg = eth_from_hex(value, &packet);
> >              if (error_msg) {
> > diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> > index 8a6c54e1da0f..4370cb5221fc 100644
> > --- a/lib/ofp-print.c
> > +++ b/lib/ofp-print.c
> > @@ -256,9 +256,9 @@ ofp_print_packet_out(struct ds *string, const struct 
> > ofp_header *oh,
> >      if (po.buffer_id == UINT32_MAX) {
> >          ds_put_format(string, " data_len=%"PRIuSIZE, po.packet_len);
> >          if (verbosity > 0 && po.packet_len > 0) {
> > -            /* Packet Out can only carry Ethernet packets. */
> > +            ovs_be32 po_packet_type = po.flow_metadata.flow.packet_type;
> >              char *packet = ofp_packet_to_string(po.packet, po.packet_len,
> > -                                                htonl(PT_ETH));
> > +                                                po_packet_type);
> >              ds_put_char(string, '\n');
> >              ds_put_cstr(string, packet);
> >              free(packet);
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index 344b66a30d0c..0c768aed9462 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -4258,6 +4258,7 @@ ofputil_decode_packet_out(struct ofputil_packet_out 
> > *po,
> >          if (error) {
> >              return error;
> >          }
> > +        match_set_packet_type(&po->flow_metadata, htonl(PT_ETH));
> >          match_set_in_port(&po->flow_metadata, in_port);
> > 
> >          error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len),
> > @@ -4271,6 +4272,7 @@ ofputil_decode_packet_out(struct ofputil_packet_out 
> > *po,
> >          const struct ofp10_packet_out *opo = ofpbuf_pull(&b, sizeof *opo);
> > 
> >          po->buffer_id = ntohl(opo->buffer_id);
> > +        match_set_packet_type(&po->flow_metadata, htonl(PT_ETH));
> >          match_set_in_port(&po->flow_metadata, 
> > u16_to_ofp(ntohs(opo->in_port)));
> > 
> >          error = ofpacts_pull_openflow_actions(&b, ntohs(opo->actions_len),
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index ad94db514c47..7541af0b25c8 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -3475,6 +3475,9 @@ ofproto_packet_out_init(struct ofproto *ofproto,
> >      /* Ensure that the L3 header is 32-bit aligned. */
> >      opo->packet = dp_packet_clone_data_with_headroom(po->packet,
> >                                                       po->packet_len, 2);
> > +    /* Take the received packet_tpye as packet_type of the packet. */
> > +    opo->packet->packet_type = po->flow_metadata.flow.packet_type;
> > +
> >      /* Store struct flow. */
> >      opo->flow = xmalloc(sizeof *opo->flow);
> >      *opo->flow = po->flow_metadata.flow;
> > diff --git a/tests/ofproto.at b/tests/ofproto.at
> > index 25aa00aff4a2..d466ae54cc61 100644
> > --- a/tests/ofproto.at
> > +++ b/tests/ofproto.at
> > @@ -4136,6 +4136,91 @@ OFPT_BARRIER_REPLY:
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> > 
> > +dnl This test checks that 1.5 packet_out is properly encoded/decoded.
> > +AT_SETUP([ofproto - packet-out with set_field metadata (OpenFlow 1.5)])
> > +OVS_VSWITCHD_START
> > +
> > +# Start a monitor listening for packet-ins.
> > +AT_CHECK([ovs-ofctl -P standard -O OpenFlow13 monitor br0 --detach 
> > --no-chdir --pidfile])
> > +ovs-appctl -t ovs-ofctl ofctl/send 0409000c0123456700000080
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
> > +AT_CAPTURE_FILE([monitor.log])
> > +
> > +# Send a packet-out with a couple of set-field action to set some 
> > metadata, and forward to controller
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 packet-out br0 CONTROLLER 
> > 'set_field:0xfafafafa5a5a5a5a->metadata, controller'
> > '0001020304050010203040501234'])
> > +
> > +# Stop the monitor and check its output.
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl exit
> > +
> > +AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
> > +OFPT_PACKET_IN (OF1.3): total_len=14 
> > metadata=0xfafafafa5a5a5a5a,in_port=CONTROLLER (via action) data_len=14
> > (unbuffered)
> > +vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
> > +OFPT_BARRIER_REPLY (OF1.3):
> > +])
> > +
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > +
> > +dnl This test checks that packet_type PT_ETH is properly encoded/decoded 
> > in 1.5 packet_out.
> > +AT_SETUP([ofproto - packet-out with set_field metadata with packet_type 
> > PT_ETH (OpenFlow 1.5)])
> > +OVS_VSWITCHD_START
> > +
> > +# Start a monitor listening for packet-ins.
> > +AT_CHECK([ovs-ofctl -P standard -O OpenFlow13 monitor br0 --detach 
> > --no-chdir --pidfile])
> > +ovs-appctl -t ovs-ofctl ofctl/send 0409000c0123456700000080
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
> > +AT_CAPTURE_FILE([monitor.log])
> > +
> > +# Send a packet-out with a couple of set-field action to set some 
> > metadata, and forward to controller
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 packet-out br0 "in_port=controller 
> > packet=0001020304050010203040501234
> > packet_type(0,0x0) 
> > actions=set_field:0xfafafafa5a5a5a5a->metadata,controller"])
> > +
> > +# Stop the monitor and check its output.
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl exit
> > +
> > +AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
> > +OFPT_PACKET_IN (OF1.3): total_len=14 
> > metadata=0xfafafafa5a5a5a5a,in_port=CONTROLLER (via action) data_len=14
> > (unbuffered)
> > +vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
> > +OFPT_BARRIER_REPLY (OF1.3):
> > +])
> > +
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > +
> > +dnl This test checks that packet_type PT_IPV4 is properly encoded/decoded 
> > in 1.5 packet_out.
> > +AT_SETUP([ofproto - packet-out with set_field metadata with packet_type 
> > PT_IPV4 on PTAP bridge (OpenFlow 1.5)])
> > +OVS_VSWITCHD_START
> > +
> > +AT_CHECK([
> > +    ovs-vsctl set bridge br0 other-config:legacy-l3-pipeline=false
> > +], [0])
> > +
> > +# Start a monitor listening for packet-ins.
> > +AT_CHECK([ovs-ofctl -P standard -O OpenFlow13 monitor br0 --detach 
> > --no-chdir --pidfile])
> > +ovs-appctl -t ovs-ofctl ofctl/send 0409000c0123456700000080
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
> > +AT_CAPTURE_FILE([monitor.log])
> > +
> > +# Send a packet-out with a couple of set-field action to set some 
> > metadata, and forward to controller
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 packet-out br0 "in_port=controller
> > packet=4500002012344000ff1155670a0000140a00001e006400c8000cea78ffffffff 
> > packet_type(1,0x800)
> > actions=set_field:0xfafafafa5a5a5a5a->metadata,controller"])
> > +
> > +# Stop the monitor and check its output.
> > +ovs-appctl -t ovs-ofctl ofctl/barrier
> > +ovs-appctl -t ovs-ofctl exit
> > +
> > +AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
> > +OFPT_PACKET_IN (OF1.3): total_len=32 
> > packet_type=(1,0x800),metadata=0xfafafafa5a5a5a5a,in_port=CONTROLLER (via
> > action) data_len=32 (unbuffered)
> > +packet_type=(1,0x800),nw_src=10.0.0.20,nw_dst=10.0.0.30,nw_proto=17,nw_tos=0,nw_ecn=0,nw_ttl=255,tp_src=100,tp_dst=
> > 200 udp_csum:ea78
> > +OFPT_BARRIER_REPLY (OF1.3):
> > +])
> > +
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > +
> >  dnl This test checks that metadata is encoded in packet_in structures,
> >  dnl supported by NXAST.
> >  AT_SETUP([ofproto - packet-out with metadata (NXM)])
> > diff --git a/tests/system-userspace-packet-type-aware.at 
> > b/tests/system-userspace-packet-type-aware.at
> > index ec2514cbbde9..8ecde6648b30 100644
> > --- a/tests/system-userspace-packet-type-aware.at
> > +++ b/tests/system-userspace-packet-type-aware.at
> > @@ -72,7 +72,7 @@ AT_CHECK([
> >      ovs-vsctl add-br br-in1 -- \
> >          set bridge br-in1 datapath_type=netdev fail-mode=standalone
> >      ovs-vsctl add-br br-in2 -- \
> > -        set bridge br-in2 datapath_type=netdev fail-mode=standalone 
> > other-config:packet-type-aware=true
> > +        set bridge br-in2 datapath_type=netdev fail-mode=standalone 
> > other-config:legacy-l3-pipeline=true
> >      ovs-vsctl add-br br-in3 -- \
> >          set bridge br-in3 datapath_type=netdev fail-mode=standalone
> >      ovs-vsctl add-br br-p1 -- \
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index d7e37e85fb92..ad862efe3e39 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -2206,6 +2206,7 @@ ofctl_packet_out(struct ovs_cmdl_context *ctx)
> >                            str_to_port_no(ctx->argv[1], ctx->argv[2]));
> >          po.ofpacts = ofpacts.data;
> >          po.ofpacts_len = ofpacts.size;
> > +        po.flow_metadata.flow.packet_type = htonl(PT_ETH);
> > 
> >          protocol = open_vconn_for_flow_mod(ctx->argv[1], &vconn,
> >                                             usable_protocols);
> > --
> > 2.10.2
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to