You didn't mention that part.  It makes a difference.

It's deceptive to use the name OFPAT_ENCAP_GTP because the OFPAT_ prefix
implies that this is a standard action.  Please choose your own prefix.

On Thu, Mar 17, 2016 at 05:30:26PM +0530, Ajmer Singh wrote:
> Hi Ben,
> 
> This new "ENCAP_GTP" action structure will be used as a payload in below
> action experimenter header structure.
> also the new structure is a multiple of 8 Bytes as per openflow
> specification.
> 
> An Experimenter action uses the following structure and fields:
> /* Action header for OFPAT_EXPERIMENTER.
> * The rest of the body is experimenter-defined. */
> *struct ofp_action_experimenter_header* {
>      uint16_t type; /* OFPAT_EXPERIMENTER. */
>      uint16_t len; /* Length is a multiple of 8. */
>      uint32_t experimenter; /* Experimenter ID which takes the same form as
> in struct ofp_experimenter_header. */
>      //action payload
>      *struct ofp_encap_gtp* {
>        uint16_t type;                /* OFPAT_ENCAP_GTP */
>        uint16_t len;                 /* Length is 16. */
>        uint32_t teid;                /* tunnel id for GTP */
>        uint32_t src_ip;              /* Source IP for external IP header */
>        uint32_t dst_ip;              /* Destination IP for external IP
> header
> */
>        unsigned char src_ipv6[16];   /* Source IP for external IPV6 header
> */
>        unsigned char dst_ipv6[16];   /* Destination IP for external IPV6
> header */
>  }
> };
> 
> 
> Could you please suggest what condition it violates in openflow
> specification?
> 
> Regards,
> Ajmer
> 
> On Thu, Mar 17, 2016 at 4:30 AM, Ben Pfaff <b...@ovn.org> wrote:
> 
> > Your action structures are wrong because they are not based on the
> > OpenFlow experimenter action format.  Please refer to the OpenFlow
> > specification for details.
> >
> > On Wed, Mar 16, 2016 at 06:06:30PM +0530, Ajmer Singh wrote:
> > > Hi Ben,
> > >
> > > I have started adding OFPAT_ENCAP_GTP action in ofp-actions.c file as
> > > following:
> > > taking reference of already added action "OFPAT_ENQUEUE". I have prepared
> > > the structure of Action, also written encode and decode for this new
> > > action. but not able to understand how its *parse_ENCAP_GTP* and
> > > *format_ENCAP_GTP* will be written.
> > >
> > > Could you guide what need to take care while writing parse and format for
> > > the action?
> > >
> > >
> > > /* encap gtp action. */
> > > struct ofp_encap_gtp {
> > >     uint16_t type;                /* OFPAT_ENCAP_GTP */
> > >     uint16_t len;                 /* Length is 16. */
> > >     uint32_t teid;                /* tunnel id for GTP */
> > >     uint32_t src_ip;              /* Source IP for external IP header */
> > >     uint32_t dst_ip;              /* Destination IP for external IP
> > header
> > > */
> > >     unsigned char src_ipv6[16];   /* Source IP for external IPV6 header
> > */
> > >     unsigned char dst_ipv6[16];   /* Destination IP for external IPV6
> > > header */
> > > };
> > > OFP_ASSERT(sizeof(struct ofp_strip_gtp) == 48);
> > >
> > > static enum ofperr
> > > *decode_OFPAT_RAW_ENCAP_GTP*(const struct ofp_encap_gtp *oegtp,
> > >                            struct ofpbuf *out)
> > > {
> > >     struct ofpact_encap_gtp *encapgtp;
> > >     uint32_t dest_ip = ntohl(oegtp->dst_ip);
> > >
> > >     encapgtp = ofpact_put_ENCAP_GTP(out);
> > >     encapgtp->teid = ntohl(oegtp->teid);
> > >     if(dest_ip != 0){
> > >        encapgtp->src_ip = ntohl(oegtp->src_ip);
> > >        encapgtp->dst_ip = ntohl(oegtp->dst_ip);
> > >     }else{
> > >        memcpy(&(encapgtp->src_ipv6), &(oegtp->src_ipv6), 16);
> > >        memcpy(&(encapgtp->dst_ipv6), &(oegtp->dst_ipv6), 16);
> > >     }
> > >     return 0;
> > > }
> > >
> > > static void
> > > *encode_ENCAP_GTP*(const struct ofpact_encap_gtp *encapgtp,
> > >                enum ofp_version ofp_version, struct ofpbuf *out)
> > > {
> > >     struct ofp_encap_gtp *oae;
> > >     uint32_t dest_ip = htonl(encapgtp->dst_ip);
> > >
> > >     oae = put_OFPACT10_ENCAP_GTP(out);
> > >     oae->teid = htonl(encapgtp->teid);
> > >     if(dest_ip != 0){
> > >        oae->src_ip = htonl(encapgtp->src_ip);
> > >        oae->dst_ip = htonl(encapgtp->dst_ip);
> > >     }else{
> > >        memcpy(&(oae->src_ipv6), &(encapgtp->src_ipv6), 16);
> > >        memcpy(&(oae->dst_ipv6), &(encapgtp->dst_ipv6), 16);
> > >     }
> > > }
> > >
> > > static char * OVS_WARN_UNUSED_RESULT
> > > *parse_ENCAP_GTP(*char *arg, struct ofpbuf *ofpacts,
> > >               enum ofputil_protocol *usable_protocols OVS_UNUSED)
> > > {
> > >     char *sp = NULL;
> > >     char *port = strtok_r(arg, ":q,", &sp);
> > >     char *queue = strtok_r(NULL, "", &sp);
> > >     struct ofpact_enqueue *enqueue;
> > >
> > >     if (port == NULL || queue == NULL) {
> > >         return xstrdup("\"enqueue\" syntax is \"enqueue:PORT:QUEUE\" or "
> > >                        "\"enqueue(PORT,QUEUE)\"");
> > >     }
> > >
> > >     enqueue = ofpact_put_ENQUEUE(ofpacts);
> > >     if (!ofputil_port_from_string(port, &enqueue->port)) {
> > >         return xasprintf("%s: enqueue to unknown port", port);
> > >     }
> > >     return str_to_u32(queue, &enqueue->queue);
> > > }
> > >
> > > static void
> > > *format_**ENCAP_GTP*(const struct ofpact_enqueue *a, struct ds *s)
> > > {
> > >     ds_put_format(s, "enqueue:");
> > >     ofputil_format_port(a->port, s);
> > >     ds_put_format(s, ":%"PRIu32, a->queue);
> > > }
> > >
> > > Regards,
> > > Ajmer
> > >
> > > On Tue, Mar 15, 2016 at 8:55 PM, Ben Pfaff <b...@ovn.org> wrote:
> > >
> > > > Please don't drop the mailing list.
> > > >
> > > > OK, so you'll have to add code to support the new vendor actions, some
> > > > in build-aux/extract-ofp-msgs, some in ofp-actions.c.
> > > >
> > > > If it's your vendor code then you can use any types you want.
> > > >
> > > > On Tue, Mar 15, 2016 at 11:36:20AM +0530, Ajmer Singh wrote:
> > > > > Hi Ben,
> > > > >
> > > > > With all due respect! I have read the entire comments for this data
> > type
> > > > > but have doubt for below two comments.
> > > > >
> > > > >  #The vendor is OF for standard OpenFlow actions, NX for Nicira
> > > > >  *           extension actions.  (Support for other vendors can be
> > added,
> > > > > but
> > > > >  *           it can't be done just based on a vendor ID definition
> > alone
> > > > >  *           because OpenFlow doesn't define a standard way to
> > specify a
> > > > >  *           subtype for vendor actions, so other vendors might do it
> > > > > different
> > > > >  *           from Nicira.)
> > > > >
> > > > > #The type, in parentheses, is the action type number (for standard
> > > > >  *           OpenFlow actions) or subtype (for vendor extension
> > actions).
> > > > >
> > > > > We are defining two new actions for GTP (STRIP & ENCAP). As per my
> > > > > understanding these are not standard OpenFlow actions that's why OF
> > will
> > > > > not be the vendor, then question is who will? what should be the
> > value of
> > > > > Vendor here. NX will definitely not.
> > > > >
> > > > > Regarding type, I think these values don't match with standard
> > OpenFlow
> > > > > actions structure defined in Openflow specification. then what
> > should be
> > > > > the value in our case.
> > > > >
> > > > > Regards,
> > > > > Ajmer
> > > > >
> > > > >
> > > > > On Mon, Mar 14, 2016 at 8:55 PM, Ben Pfaff <b...@ovn.org> wrote:
> > > > >
> > > > > > On Mon, Mar 14, 2016 at 06:47:34PM +0530, Ajmer Singh wrote:
> > > > > > > Hi,
> > > > > > > I have a requirement of adding GTP headers in open Vswitch source
> > > > code.
> > > > > > We
> > > > > > > need to define 2 new actions: STRIP_GTP and ENCAP_GTP. these are
> > the
> > > > > > > extensions.
> > > > > > >
> > > > > > > As per openflow specifications 1.0 and 1.1. If we add our 2 new
> > > > actions
> > > > > > > then following structure will look like as follows.
> > > > > > >
> > > > > > > enum ofp_action_type {
> > > > > > >     OFPAT_OUTPUT = 0,        /* Output to switch port. */
> > > > > > >     OFPAT_COPY_TTL_OUT = 11, /* Copy TTL "outwards" -- from
> > > > > > > next-to-outermost
> > > > > > >                                 to outermost */
> > > > > > >     OFPAT_COPY_TTL_IN = 12,  /* Copy TTL "inwards" -- from
> > outermost
> > > > to
> > > > > > >                                 next-to-outermost */
> > > > > > >     OFPAT_SET_MPLS_TTL = 15, /* MPLS TTL */
> > > > > > >     OFPAT_DEC_MPLS_TTL = 16, /* Decrement MPLS TTL */
> > > > > > >     OFPAT_PUSH_VLAN = 17,    /* Push a new VLAN tag */
> > > > > > >     OFPAT_POP_VLAN = 18,     /* Pop the outer VLAN tag */
> > > > > > >     OFPAT_PUSH_MPLS = 19,    /* Push a new MPLS tag */
> > > > > > >     OFPAT_POP_MPLS = 20,     /* Pop the outer MPLS tag */
> > > > > > >     OFPAT_SET_QUEUE = 21,    /* Set queue id when outputting to a
> > > > port */
> > > > > > >     OFPAT_GROUP = 22,        /* Apply group. */
> > > > > > >     OFPAT_SET_NW_TTL = 23,   /* IP TTL. */
> > > > > > >     OFPAT_DEC_NW_TTL = 24,   /* Decrement IP TTL. */
> > > > > > >     OFPAT_SET_FIELD = 25,    /* Set a header field using OXM TLV
> > > > format.
> > > > > > */
> > > > > > >     OFPAT_PUSH_PBB = 26,     /*Push a new PBB service tag
> > (I-TAG) */
> > > > > > >     OFPAT_POP_PBB = 27,      /* Pop the outer PBB service tag
> > > > (I-TAG) */
> > > > > > >     OFPAT_STRIP_GTP = 0xfffd,  /* Strip GTP to get inner
> > payload.  */
> > > > > > >     OFPAT_ENCAP_GTP = 0xfffe,  /* Add external IP header, UDP
> > header
> > > > and
> > > > > > > GTP header */
> > > > > > >     OFPAT_EXPERIMENTER = 0xffff
> > > > > > > };
> > > > > > > but I try to support this in open Vswitch code. and I find there
> > is
> > > > one
> > > > > > raw
> > > > > > > action structure named as ofp_raw_action_type
> > (/lib/ofp-actions.c),
> > > > > > defined
> > > > > > > as.
> > > > > > >
> > > > > > > enum ofp_raw_action_type {
> > > > > > > /* ## ----------------- ## */
> > > > > > > /* ## Standard actions. ## */
> > > > > > > /* ## ----------------- ## */
> > > > > > >
> > > > > > >     /* OF1.0(0): struct ofp10_action_output. */
> > > > > > >     OFPAT_RAW10_OUTPUT,
> > > > > > >     /* OF1.1+(0): struct ofp11_action_output. */
> > > > > > >     OFPAT_RAW11_OUTPUT,
> > > > > > > }
> > > > > > >
> > > > > > > now I need to add Raw enum constants for these 2 new actions
> > > > (STRIP_GTP
> > > > > > and
> > > > > > > ENCAP_GTP) in this structure. we have same structures for these
> > 2 new
> > > > > > > actions across all openflow versions. but facing issue in
> > defining
> > > > > > comment
> > > > > > > line before these constants.
> > > > > > > /* OF1.0(0): struct ofp10_action_output. */
> > > > > > > here, OF->for standard open flow actions
> > > > > > > 1.0->openflow protocol version
> > > > > > > (0)->action type number in specification
> > > > > > > struct ofp10_action_output -> corresponding structure for this
> > > > action.
> > > > > > >
> > > > > > > Here, I m facing issue in defining these comments since these
> > two new
> > > > > > > actions are not standard openflow actions. these are vendor
> > actions.
> > > > then
> > > > > > > how to write the comments for these new actions.
> > > > > >
> > > > > > Did you read the large comment just above enum
> > ofp_raw_action_type?  It
> > > > > > has lots of information.
> > > > > >
> > > >
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to