On Wed, Mar 29, 2017 at 12:14:24AM +0200, Sebastian Benoit wrote:
> Job Snijders([email protected]) on 2017.03.28 14:12:42 -0500:
> > 
> >     match from any \
> >         set { ext-community bovs not-found \
> 
> do other implementations call this "bovs"?  dont get me wrong, i think
> junipers origin-validation-state-valid is too long.

¯\_(ツ)_/¯, alternatively, "ovs"?

> >     match from any \
> >         prefix 2a02:898::/32 source-as 8283 \
> >         set { ext-community bovs valid }
> 
> So if we want to implement rfc6810/rfc6811 (BGP Prefix Origin
> Validation), would this syntax become redundant? Should we not
> implement 
> 
>   match from any validation-state valid ...
> 
> at this point, and hide the community behind that? And when we get
> Origin Validation, its transparent in the ruleset where the
> information came from, from our own lookup or through the community.
> 
> Just a thought, not something that makes the config a bit cleaner.

I do not think it will be redundant, we should clearly separate:

    "a well-known community which can be set by anyone", and
    "the validation state internal to the daemon".

I'd consider it an important feature to be able to match based on what a
bgp community alleges about the validation state, and what the actual
validation state is based on the daemon's own lookup.

Also, through this patch one is now able to match on the community
itself, which also means one can now scrub the community when received
from other (untrusted) parties.

So, I recommend against conflation.

> > --- bgpd/util.c     24 Jan 2017 04:22:42 -0000      1.24
> > +++ bgpd/util.c     28 Mar 2017 19:01:34 -0000
> > [ snip ]
> > +   case EXT_COMMUNITY_VALIDATION_STATE:
> > +           return ("bovs"); /* RFC 8097 */
> 
> spell out what bovs means?

ack.

> > +log_ext_bovs_value(u_int8_t value)
> > +{
> > [ snip ]
> > +}
> 
> this function is only used in bgpctl. it should be moved there.  we
> are trying to get rid of the reacharound *ctl programs are doing, so
> you should not add another function.

ack.

On Thu, Mar 30, 2017 at 12:48:45AM +0200, Claudio Jeker wrote:
> On Tue, Mar 28, 2017 at 02:12:42PM -0500, Job Snijders wrote:
> >  int
> > +parse_bgp_validation_state(char *state)
> > [ snip ]
> 
> Is there an expectation that this list will grow? I find it a bit
> overkill to use bsearch for 3 values. I understand we use similar
> constructs for other tables. Doing 3 strcmp in a row would have been
> easier I think.

I don't expect the list to grow any time soon. I've changed it.

Kind regards,

Job


Index: bgpctl/bgpctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.193
diff -u -p -r1.193 bgpctl.c
--- bgpctl/bgpctl.c     23 Jan 2017 23:38:51 -0000      1.193
+++ bgpctl/bgpctl.c     3 Apr 2017 07:24:37 -0000
@@ -2,7 +2,7 @@
 
 /*
  * Copyright (c) 2003 Henning Brauer <[email protected]>
- * Copyright (c) 2016 Job Snijders <[email protected]>
+ * Copyright (c) 2016, 2017 Job Snijders <[email protected]>
  * Copyright (c) 2016 Peter Hessler <[email protected]>
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -84,6 +84,7 @@ void           show_rib_detail(struct ctl_show_r
 void            show_attr(void *, u_int16_t);
 void            show_community(u_char *, u_int16_t);
 void            show_large_community(u_char *, u_int16_t);
+const char     *log_ext_ovs_value(u_int8_t);
 void            show_ext_community(u_char *, u_int16_t);
 char           *fmt_mem(int64_t);
 int             show_rib_memory_msg(struct imsg *);
@@ -1578,6 +1579,24 @@ show_large_community(u_char *data, u_int
        }
 }
 
+const char *
+log_ext_ovs_value(u_int8_t value)
+{
+       static char s[6];
+
+       switch (value) {
+               case EXT_COMMUNITY_VALIDATION_STATE_VALID:
+               return ("valid");
+       case EXT_COMMUNITY_VALIDATION_STATE_NOTFOUND:
+               return ("not-found");
+       case EXT_COMMUNITY_VALIDATION_STATE_INVALID:
+               return ("invalid");
+       default:
+               snprintf(s, sizeof(s), "[%u?]", value);
+               return (s);
+       }
+}
+
 void
 show_ext_community(u_char *data, u_int16_t len)
 {
@@ -1585,7 +1604,7 @@ show_ext_community(u_char *data, u_int16
        struct in_addr  ip;
        u_int32_t       as4, u32;
        u_int16_t       i, as2, u16;
-       u_int8_t        type, subtype;
+       u_int8_t        type, subtype, state;
 
        if (len & 0x7)
                return;
@@ -1618,6 +1637,13 @@ show_ext_community(u_char *data, u_int16
                        ext = betoh64(ext) & 0xffffffffffffLL;
                        printf("%s 0x%llx", log_ext_subtype(subtype), ext);
                        break;
+               case EXT_COMMUNITY_NON_TRANS_OPAQUE:
+                       if (subtype == EXT_COMMUNITY_VALIDATION_STATE) {
+                               state = data[i + 7];
+                               printf("%s %s", log_ext_subtype(subtype),
+                                   log_ext_ovs_value(state));
+                               break;
+                       }
                default:
                        memcpy(&ext, data + i, sizeof(ext));
                        printf("0x%llx", betoh64(ext));
Index: bgpd/bgpd.8
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v
retrieving revision 1.52
diff -u -p -r1.52 bgpd.8
--- bgpd/bgpd.8 19 Feb 2017 11:38:24 -0000      1.52
+++ bgpd/bgpd.8 3 Apr 2017 07:24:37 -0000
@@ -390,6 +390,17 @@ control socket
 .%R draft-ietf-idr-shutdown
 .%T BGP Administrative Shutdown Communication
 .Re
+.Pp
+.Rs
+.%A P. Mohapatra
+.%A K. Patel
+.%A J. Scudder
+.%A D. Ward
+.%A R. Bush
+.%D March 2017
+.%R RFC 8097
+.%T BGP Prefix Origin Validation State Extended Community
+.Re
 .Sh HISTORY
 The
 .Nm
Index: bgpd/bgpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.152
diff -u -p -r1.152 bgpd.conf.5
--- bgpd/bgpd.conf.5    13 Jan 2017 18:59:12 -0000      1.152
+++ bgpd/bgpd.conf.5    3 Apr 2017 07:24:37 -0000
@@ -1195,6 +1195,11 @@ which is expanded to the current neighbo
 .Ic ext-community
 .Ar subtype Ar numvalue
 .Xc
+.It Xo
+.Ic ext-community
+.Ar ovs
+.Pq Ic valid | not-found | invalid
+.Xc
 This rule applies only to
 .Em UPDATES
 where the
@@ -1456,6 +1461,11 @@ to do wildcard matching.
 .Ic ext-community Op Ar delete
 .Ar subtype Ar numvalue
 .Xc
+.It Xo
+.Ic ext-community Op Ar delete
+.Ar ovs
+.Pq Ic valid | not-found | invalid
+.Xc
 Set or delete the
 .Em Extended Community
 AS path attribute.
@@ -1481,6 +1491,7 @@ odi      OSPF Domain Identifier
 ort      OSPF Route Type
 ori      OSPF Router ID
 bdc      BGP Data Collection
+ovs      BGP Origin Validation State
 .Ed
 .Pp
 Not all type and subtype value pairs are allowed by IANA and the parser
Index: bgpd/bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.300
diff -u -p -r1.300 bgpd.h
--- bgpd/bgpd.h 25 Jan 2017 00:11:07 -0000      1.300
+++ bgpd/bgpd.h 3 Apr 2017 07:24:37 -0000
@@ -755,7 +755,8 @@ struct filter_peers {
 #define EXT_COMMUNITY_TWO_AS           0       /* 2 octet AS specific */
 #define EXT_COMMUNITY_IPV4             1       /* IPv4 specific */
 #define EXT_COMMUNITY_FOUR_AS          2       /* 4 octet AS specific */
-#define EXT_COMMUNITY_OPAQUE           3       /* opaque ext community */
+#define EXT_COMMUNITY_OPAQUE           3       /* transitive opaque ext */
+#define EXT_COMMUNITY_NON_TRANS_OPAQUE 43      /* non-transitive opaque ext */
 /* sub types */
 #define EXT_COMMUNITY_ROUTE_TGT                2       /* RFC 4360 & RFC4364 */
 #define EXT_COMMUNITY_ROUTE_ORIG       3       /* RFC 4360 & RFC4364 */
@@ -763,6 +764,11 @@ struct filter_peers {
 #define EXT_COMMUNITY_OSPF_RTR_TYPE    6       /* RFC 4577 */
 #define EXT_COMMUNITY_OSPF_RTR_ID      7       /* RFC 4577 */
 #define EXT_COMMUNITY_BGP_COLLECT      8       /* RFC 4384 */
+#define EXT_COMMUNITY_VALIDATION_STATE 0       /* RFC 8097 */
+/* RFC 8097 validation states */
+#define EXT_COMMUNITY_VALIDATION_STATE_VALID   0
+#define EXT_COMMUNITY_VALIDATION_STATE_NOTFOUND        1
+#define EXT_COMMUNITY_VALIDATION_STATE_INVALID 2
 /* other handy defines */
 #define EXT_COMMUNITY_OPAQUE_MAX       0xffffffffffffULL
 #define EXT_COMMUNITY_FLAG_VALID       0x01
@@ -783,7 +789,8 @@ struct ext_comm_pairs {
        { EXT_COMMUNITY_IPV4, EXT_COMMUNITY_ROUTE_TGT, 0 },             \
        { EXT_COMMUNITY_IPV4, EXT_COMMUNITY_ROUTE_ORIG, 0 },            \
        { EXT_COMMUNITY_IPV4, EXT_COMMUNITY_OSPF_RTR_ID, 0 },           \
-       { EXT_COMMUNITY_OPAQUE, EXT_COMMUNITY_OSPF_RTR_TYPE, 0 }        \
+       { EXT_COMMUNITY_OPAQUE, EXT_COMMUNITY_OSPF_RTR_TYPE, 0 },       \
+       { EXT_COMMUNITY_NON_TRANS_OPAQUE, EXT_COMMUNITY_VALIDATION_STATE, 0 } \
 }
 
 
Index: bgpd/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.298
diff -u -p -r1.298 parse.y
--- bgpd/parse.y        22 Feb 2017 13:55:14 -0000      1.298
+++ bgpd/parse.y        3 Apr 2017 07:24:37 -0000
@@ -5,7 +5,7 @@
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2001 Daniel Hartmeier.  All rights reserved.
  * Copyright (c) 2001 Theo de Raadt.  All rights reserved.
- * Copyright (c) 2016 Job Snijders <[email protected]>
+ * Copyright (c) 2016, 2017 Job Snijders <[email protected]>
  * Copyright (c) 2016 Peter Hessler <[email protected]>
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -145,6 +145,7 @@ int          parsecommunity(struct filter_commu
 int64_t         getlargecommunity(char *);
 int             parselargecommunity(struct filter_largecommunity *, char *);
 int             parsesubtype(char *);
+int             parse_bgp_validation_state(char *);
 int             parseextvalue(char *, u_int32_t *);
 int             parseextcommunity(struct filter_extcommunity *, char *,
                    char *);
@@ -3022,6 +3023,19 @@ parselargecommunity(struct filter_largec
 }
 
 int
+parse_bgp_validation_state(char *s)
+{
+       if (strcmp(s, "invalid") == 0)
+               return (EXT_COMMUNITY_VALIDATION_STATE_INVALID);
+       else if (strcmp(s, "not-found") == 0)
+               return (EXT_COMMUNITY_VALIDATION_STATE_NOTFOUND);
+       else if (strcmp(s, "valid") == 0)
+               return (EXT_COMMUNITY_VALIDATION_STATE_VALID);
+       else
+               return (-1);
+}
+
+int
 parsesubtype(char *type)
 {
        /* this has to be sorted always */
@@ -3030,6 +3044,7 @@ parsesubtype(char *type)
                { "odi",        EXT_COMMUNITY_OSPF_DOM_ID },
                { "ori",        EXT_COMMUNITY_OSPF_RTR_ID },
                { "ort",        EXT_COMMUNITY_OSPF_RTR_TYPE },
+               { "ovs",        EXT_COMMUNITY_VALIDATION_STATE },
                { "rt",         EXT_COMMUNITY_ROUTE_TGT },
                { "soo",        EXT_COMMUNITY_ROUTE_ORIG }
        };
@@ -3100,15 +3115,22 @@ parseextcommunity(struct filter_extcommu
        u_int32_t        uval;
        char            *p, *ep;
        unsigned int     i;
-       int              type, subtype;
+       int              type, state, subtype;
 
        if ((subtype = parsesubtype(t)) == -1) {
                yyerror("Bad ext-community unknown type");
                return (-1);
        }
 
-       if ((p = strchr(s, ':')) == NULL) {
-               type = EXT_COMMUNITY_OPAQUE,
+       if (subtype == EXT_COMMUNITY_VALIDATION_STATE) {
+               if ((state = parse_bgp_validation_state(s)) == -1) {
+                       yyerror("Invalid BGP Origin Validation State 
Community");
+                       return(-1);
+               }
+               type = EXT_COMMUNITY_NON_TRANS_OPAQUE;
+               c->data.ext_opaq = state;
+       } else if ((p = strchr(s, ':')) == NULL) {
+               type = EXT_COMMUNITY_OPAQUE;
                errno = 0;
                ullval = strtoull(s, &ep, 0);
                if (s[0] == '\0' || *ep != '\0') {
Index: bgpd/printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.100
diff -u -p -r1.100 printconf.c
--- bgpd/printconf.c    24 Jan 2017 04:22:42 -0000      1.100
+++ bgpd/printconf.c    3 Apr 2017 07:24:37 -0000
@@ -149,6 +149,7 @@ print_extcommunity(struct filter_extcomm
                    log_as(c->data.ext_as4.as4), c->data.ext_as.val);
                break;
        case EXT_COMMUNITY_OPAQUE:
+       case EXT_COMMUNITY_NON_TRANS_OPAQUE:
                printf("%s 0x%llx ", log_ext_subtype(c->subtype),
                    c->data.ext_opaq);
                break;
Index: bgpd/util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.24
diff -u -p -r1.24 util.c
--- bgpd/util.c 24 Jan 2017 04:22:42 -0000      1.24
+++ bgpd/util.c 3 Apr 2017 07:24:37 -0000
@@ -154,6 +154,8 @@ log_ext_subtype(u_int8_t subtype)
                return ("ori"); /* ospf router id */
        case EXT_COMMUNITY_BGP_COLLECT:
                return ("bdc"); /* bgp data collection */
+       case EXT_COMMUNITY_VALIDATION_STATE:
+               return ("ovs"); /* origin validation state, RFC 8097 */
        default:
                snprintf(etype, sizeof(etype), "[%u]", subtype);
                return (etype);

Reply via email to