pbrd: add explicit 'family' field for rules

    In the netlink-mediated kernel dataplane, each rule is stored
    in either an IPv4-specific database or an IPv6-specific database.
    PBRD opportunistically gleans each rule's address family value
    from its source or destination IP address match value (if either
    exists), or from its nexthop or nexthop-group (if it exists).

    The 'family' value is particularly needed for netlink during
    incremental rule deletion when none of the above fields remain set.

    Before now, this address family has been encoded by occult means
    in the (possibly otherwise unset) source/destination IP match
    fields in ZAPI and zebra.

    This commit documents the reasons for maintaining the 'family'
    field in the PBRD rule structure, adds a 'family' field in the
    common lib/pbr.h rule structure, and carries it explicitly in ZAPI.

Signed-off-by: G. Paul Ziemba <paulz@labn.net>

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 8ff6b63e0565011bfb7d4f2cf73cdffa6f329d2b..9784c1c837a58fd785ce1395b3e05a0d7ecec1da 100644 (file)
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -2737,6 +2737,7 @@ static void bgp_encode_pbr_rule_action(struct stream *s,
        else
                r.unique = pbra->unique;
 
+       r.family = fam;
 
        /* filter */
 

diff --git a/lib/pbr.h b/lib/pbr.h
index 1c89f2f552c863f6e2b22d46201ec537ecda2abb..61a5eb07f61d97e2b9e219637c68653b50570dd9 100644 (file)
--- a/lib/pbr.h
+++ b/lib/pbr.h
@@ -114,6 +114,7 @@ struct pbr_action {
  */
 struct pbr_rule {
        vrf_id_t vrf_id;
+       uint8_t family; /* netlink: select which rule database */
 
        uint32_t seq;
        uint32_t priority;

diff --git a/lib/zclient.c b/lib/zclient.c
index 294a78feb09550ce9effa80ea13e19a41d92e0a3..4648b285fdf17fafef9d89a5a625704c70f807c5 100644 (file)
--- a/lib/zclient.c
+++ b/lib/zclient.c
@@ -1702,6 +1702,7 @@ int zapi_pbr_rule_encode(struct stream *s, struct pbr_rule *r)
         */
        stream_putl(s, 1);
 
+       stream_putc(s, r->family);
        stream_putl(s, r->seq);
        stream_putl(s, r->priority);
        stream_putl(s, r->unique);
@@ -1723,6 +1724,7 @@ bool zapi_pbr_rule_decode(struct stream *s, struct pbr_rule *r)
 
        memset(r, 0, sizeof(*r));
 
+       STREAM_GETC(s, r->family);
        STREAM_GETL(s, r->seq);
        STREAM_GETL(s, r->priority);
        STREAM_GETL(s, r->unique);

diff --git a/pbrd/pbr_zebra.c b/pbrd/pbr_zebra.c
index 030c4c1114ea808e432c6db4ed7eff480d7b74fd..adcf449cfac00bdfce421c00a7d7b3b2e6ec0d4c 100644 (file)
--- a/pbrd/pbr_zebra.c
+++ b/pbrd/pbr_zebra.c
@@ -517,10 +517,14 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
        uint8_t family;
 
        /*
-        * There seems to be some effort in pbr_vty.c to keep the three
-        * copies of "family" equal. Not sure if the reason goes beyond
-        * ensuring consistency in ZAPI encoding. In any case, it might
-        * be handled better as an internal matter for the encoder (TBD).
+        * Opportunistic address family field is set when any of the IP
+        * address match/set fields is set, or when a NH/NHG is resolved.
+        * The value is needed by zebra for the underlying netlink
+        * messaging, particularly in delete operations, because it
+        * selects the rule database (IPv4 vs. IPv6).
+        *
+        * Historically the value has been encoded into any unused
+        * "match src/dst address" fields and picked off in zebra.
         */
        family = AF_INET;
        if (pbrms->family)
@@ -539,6 +543,8 @@ static bool pbr_encode_pbr_map_sequence(struct stream *s,
        r.priority = pbrms->ruleno;
        r.unique = pbrms->unique;
 
+       r.family = pbrms->family;
+
        /* filter */
        r.filter.filter_bm = pbrms->filter_bm;
        if (pbrms->src)