Thanks Ben! I will update the name as per your comment.

can you please answer 2nd part of the question? I have to write
encode,decode,parse and format functions for this new action. I have
updated decode and encode as per previously defined actions but find it
difficult for "parse" and format functions.

Can you please share the logic behind format and parse functions so that i
can image what type of changes should be there? like from where these
inputs or input parameters/arguments will be received?

I have taken reference of ofpact_enqueue 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,
Ajme

On Thu, Mar 17, 2016 at 9:38 PM, Ben Pfaff <b...@ovn.org> wrote:

> 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