Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ajmer Singh]

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,", );
char *queue = strtok_r(NULL, "", );
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, >port)) {
return xasprintf("%s: enqueue to unknown port", port);
}
return str_to_u32(queue, >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  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  wrote:
> >
> > > Your action structures are wrong because they 

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ben Pfaff]

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,", );
> char *queue = strtok_r(NULL, "", );
> 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, >port)) {
> return xasprintf("%s: enqueue to unknown port", port);
> }
> return str_to_u32(queue, >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  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 

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ajmer Singh]

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,", );
char *queue = strtok_r(NULL, "", );
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, >port)) {
return xasprintf("%s: enqueue to unknown port", port);
}
return str_to_u32(queue, >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  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  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 

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ben Pfaff]

On Fri, Mar 18, 2016 at 11:29:39AM +0530, Ajmer Singh wrote:
> 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?

These are for ovs-ofctl and similar, to parse and dump flows.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ajmer Singh]

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  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,", );
> > char *queue = strtok_r(NULL, "", );
> > 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, >port)) {
> > return xasprintf("%s: enqueue to unknown port", port);
> > }
> > return str_to_u32(queue, >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 

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ben Pfaff]

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  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,", );
> > > char *queue = strtok_r(NULL, "", );
> > > struct ofpact_enqueue *enqueue;
> > >
> > > if (port == NULL || queue == NULL) {
> > > return xstrdup("\"enqueue\" syntax is \"enqueue:PORT:QUEUE\" or "
> > >"\"enqueue(PORT,QUEUE)\"");
> > > }
> > >
> > > enqueue = 

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ben Pfaff]

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  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 = 0x
> > > };
> > > 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

Re: [ovs-dev] How to define enum raw constants for two new vendor actions

[Ben Pfaff]

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 = 0x
> };
> 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