Thanks for doing this change Nicholas. Acked-by: Donald Sharp <[email protected]>
On Mon, Sep 7, 2015 at 9:56 AM, Nicolas Dichtel <[email protected]> wrote: > Commit c99f3481a598 has changed the API. Now, the vrfid has been added in > the header, thus we must read it before parsing the rest of the message. > > To ease code maintenance, let's add a new function to read a zAPI header. > > Fixes: c99f3481a598 ("*: add VRF ID in the API message header") > Signed-off-by: Nicolas Dichtel <[email protected]> > --- > > Martin, could you test this patch? At least it fixes the ebgp multihop bug. > > v2: add zclient_read_header() > fix also bgpd/zlookup_read_ipv6(), bgpd/bgp_import_check() and > pimd/zclient_read_nexthop() > > bgpd/bgp_nexthop.c | 68 > ++++++++++++++++++++++-------------------------------- > lib/zclient.c | 15 ++++++++++++ > lib/zclient.h | 3 +++ > pimd/pim_zlookup.c | 51 +++++++++++++++++++--------------------- > 4 files changed, 69 insertions(+), 68 deletions(-) > > diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c > index 7336793e01e6..c74bebad86e3 100644 > --- a/bgpd/bgp_nexthop.c > +++ b/bgpd/bgp_nexthop.c > @@ -789,8 +789,9 @@ zlookup_read (void) > uint16_t length; > u_char marker; > u_char version; > - uint16_t command __attribute__((unused)); > - int nbytes __attribute__((unused)); > + uint16_t vrf_id; > + uint16_t command; > + int err; > struct in_addr raddr __attribute__((unused)); > uint32_t metric; > int i; > @@ -801,14 +802,13 @@ zlookup_read (void) > s = zlookup->ibuf; > stream_reset (s); > > - /* nbytes not being checked */ > - nbytes = stream_read (s, zlookup->sock, 2); > - length = stream_getw (s); > - > - nbytes = stream_read (s, zlookup->sock, length - 2); > - marker = stream_getc (s); > - version = stream_getc (s); > - > + err = zclient_read_header (s, zlookup->sock, &length, &marker, &version, > + &vrf_id, &command); > + if (err < 0) > + { > + zlog_err("%s: zserv_read_header() failed", __func__); > + return NULL; > + } > if (version != ZSERV_VERSION || marker != ZEBRA_HEADER_MARKER) > { > zlog_err("%s: socket %d version mismatch, marker %d, version %d", > @@ -816,9 +816,6 @@ zlookup_read (void) > return NULL; > } > > - /* XXX: not checking command */ > - command = stream_getw (s); > - > /* XXX: not doing anything with raddr */ > raddr.s_addr = stream_get_ipv4 (s); > metric = stream_getl (s); > @@ -902,11 +899,11 @@ static struct bgp_nexthop_cache * > zlookup_read_ipv6 (void) > { > struct stream *s; > - uint16_t length; > + uint16_t length, vrf_id, cmd; > u_char version, marker; > struct in6_addr raddr; > uint32_t metric; > - int i; > + int i, err; > u_char nexthop_num; > struct nexthop *nexthop; > struct bgp_nexthop_cache *bnc; > @@ -914,14 +911,13 @@ zlookup_read_ipv6 (void) > s = zlookup->ibuf; > stream_reset (s); > > - /* XXX: ignoring nbytes, see also zread_lookup */ > - stream_read (s, zlookup->sock, 2); > - length = stream_getw (s); > - > - stream_read (s, zlookup->sock, length - 2); > - marker = stream_getc (s); > - version = stream_getc (s); > - > + err = zclient_read_header (s, zlookup->sock, &length, &marker, &version, > + &vrf_id, &cmd); > + if (err < 0) > + { > + zlog_err("%s: zserv_read_header() failed", __func__); > + return NULL; > + } > if (version != ZSERV_VERSION || marker != ZEBRA_HEADER_MARKER) > { > zlog_err("%s: socket %d version mismatch, marker %d, version %d", > @@ -929,9 +925,6 @@ zlookup_read_ipv6 (void) > return NULL; > } > > - /* XXX: ignoring command */ > - stream_getw (s); > - > /* XXX: not actually doing anything with raddr */ > stream_get (&raddr, s, 16); > > @@ -1018,9 +1011,8 @@ bgp_import_check (struct prefix *p, u_int32_t > *igpmetric, > { > struct stream *s; > int ret; > - u_int16_t length, command __attribute__((unused)); > + u_int16_t length, vrf_id, command; > u_char version, marker; > - int nbytes __attribute__((unused)); > struct in_addr addr __attribute__((unused)); > struct in_addr nexthop; > u_int32_t metric = 0; > @@ -1066,16 +1058,13 @@ bgp_import_check (struct prefix *p, u_int32_t > *igpmetric, > /* Get result. */ > stream_reset (s); > > - /* Fetch length. */ > - /* XXX: not using nbytes */ > - nbytes = stream_read (s, zlookup->sock, 2); > - length = stream_getw (s); > - > - /* Fetch whole data. */ > - nbytes = stream_read (s, zlookup->sock, length - 2); > - marker = stream_getc (s); > - version = stream_getc (s); > - > + ret = zclient_read_header (s, zlookup->sock, &length, &marker, &version, > + &vrf_id, &command); > + if (ret < 0) > + { > + zlog_err("%s: zserv_read_header() failed", __func__); > + return 0; > + } > if (version != ZSERV_VERSION || marker != ZEBRA_HEADER_MARKER) > { > zlog_err("%s: socket %d version mismatch, marker %d, version %d", > @@ -1083,9 +1072,6 @@ bgp_import_check (struct prefix *p, u_int32_t > *igpmetric, > return 0; > } > > - /* XXX: not using command */ > - command = stream_getw (s); > - > /* XXX: not using addr */ > addr.s_addr = stream_get_ipv4 (s); > metric = stream_getl (s); > diff --git a/lib/zclient.c b/lib/zclient.c > index 8e443e28d26f..c1bdd8ff4f2d 100644 > --- a/lib/zclient.c > +++ b/lib/zclient.c > @@ -302,6 +302,21 @@ zclient_create_header (struct stream *s, uint16_t > command, vrf_id_t vrf_id) > stream_putw (s, command); > } > > +int > +zclient_read_header (struct stream *s, int sock, u_int16_t *size, u_char > *marker, > + u_char *version, u_int16_t *vrf_id, u_int16_t *cmd) > +{ > + if (stream_read (s, sock, ZEBRA_HEADER_SIZE) != ZEBRA_HEADER_SIZE) > + return -1; > + > + *size = stream_getw (s) - ZEBRA_HEADER_SIZE; > + *marker = stream_getc (s); > + *version = stream_getc (s); > + *vrf_id = stream_getw (s); > + *cmd = stream_getw (s); > + return 0; > +} > + > /* Send simple Zebra message. */ > static int > zebra_message_send (struct zclient *zclient, int command, vrf_id_t vrf_id) > diff --git a/lib/zclient.h b/lib/zclient.h > index 19b4f0ead6cd..3490b32092b2 100644 > --- a/lib/zclient.h > +++ b/lib/zclient.h > @@ -163,6 +163,9 @@ extern int zclient_send_message(struct zclient *); > > /* create header for command, length to be filled in by user later */ > extern void zclient_create_header (struct stream *, uint16_t, vrf_id_t); > +extern int zclient_read_header (struct stream *s, int sock, u_int16_t > *size, > + u_char *marker, u_char *version, > + u_int16_t *vrf_id, u_int16_t *cmd); > > extern struct interface *zebra_interface_add_read (struct stream *, > vrf_id_t); > diff --git a/pimd/pim_zlookup.c b/pimd/pim_zlookup.c > index 60003670cf6d..fae8f81ca1b0 100644 > --- a/pimd/pim_zlookup.c > +++ b/pimd/pim_zlookup.c > @@ -149,17 +149,18 @@ static int zclient_read_nexthop(struct zclient > *zlookup, > { > int num_ifindex = 0; > struct stream *s; > - const uint16_t MIN_LEN = 14; /* getc=1 getc=1 getw=2 getipv4=4 getc=1 > getl=4 getc=1 */ > - uint16_t length, len; > + const uint16_t MIN_LEN = 10; /* getipv4=4 getc=1 getl=4 getc=1 */ > + uint16_t length; > u_char marker; > u_char version; > + uint16_t vrf_id; > uint16_t command; > int nbytes; > struct in_addr raddr; > uint8_t distance; > uint32_t metric; > int nexthop_num; > - int i; > + int i, err; > > if (PIM_DEBUG_ZEBRA) { > char addr_str[100]; > @@ -172,41 +173,37 @@ static int zclient_read_nexthop(struct zclient > *zlookup, > s = zlookup->ibuf; > stream_reset(s); > > - nbytes = stream_read(s, zlookup->sock, 2); > - if (nbytes < 2) { > - zlog_err("%s %s: failure reading zclient lookup socket: nbytes=%d", > - __FILE__, __PRETTY_FUNCTION__, nbytes); > + err = zclient_read_header (s, zlookup->sock, &length, &marker, &version, > + &vrf_id, &command); > + if (err < 0) { > + zlog_err("%s %s: zclient_read_header() failed", > + __FILE__, __PRETTY_FUNCTION__); > zclient_lookup_failed(zlookup); > return -1; > } > - length = stream_getw(s); > - > - len = length - 2; > > - if (len < MIN_LEN) { > + if (length < MIN_LEN) { > zlog_err("%s %s: failure reading zclient lookup socket: len=%d < > MIN_LEN=%d", > - __FILE__, __PRETTY_FUNCTION__, len, MIN_LEN); > + __FILE__, __PRETTY_FUNCTION__, length, MIN_LEN); > zclient_lookup_failed(zlookup); > return -2; > } > - > - nbytes = stream_read(s, zlookup->sock, len); > - if (nbytes < (length - 2)) { > + > + nbytes = stream_read(s, zlookup->sock, length); > + if (nbytes < length) { > zlog_err("%s %s: failure reading zclient lookup socket: nbytes=%d < > len=%d", > - __FILE__, __PRETTY_FUNCTION__, nbytes, len); > + __FILE__, __PRETTY_FUNCTION__, nbytes, length); > zclient_lookup_failed(zlookup); > return -3; > } > - marker = stream_getc(s); > - version = stream_getc(s); > > if (version != ZSERV_VERSION || marker != ZEBRA_HEADER_MARKER) { > zlog_err("%s: socket %d version mismatch, marker %d, version %d", > __func__, zlookup->sock, marker, version); > + zclient_lookup_failed(zlookup); > return -4; > } > > - command = stream_getw(s); > if (command != ZEBRA_IPV4_NEXTHOP_LOOKUP_MRIB) { > zlog_err("%s: socket %d command mismatch: %d", > __func__, zlookup->sock, command); > @@ -236,19 +233,19 @@ static int zclient_read_nexthop(struct zclient > *zlookup, > return -6; > } > > - len -= MIN_LEN; > + length -= MIN_LEN; > > for (i = 0; i < nexthop_num; ++i) { > enum nexthop_types_t nexthop_type; > > - if (len < 1) { > + if (length < 1) { > zlog_err("%s: socket %d empty input expecting nexthop_type: len=%d", > - __func__, zlookup->sock, len); > + __func__, zlookup->sock, length); > return -7; > } > > nexthop_type = stream_getc(s); > - --len; > + --length; > > switch (nexthop_type) { > case ZEBRA_NEXTHOP_IFINDEX: > @@ -263,13 +260,13 @@ static int zclient_read_nexthop(struct zclient > *zlookup, > return num_ifindex; > } > if (nexthop_type == ZEBRA_NEXTHOP_IPV4_IFINDEX) { > - if (len < 4) { > + if (length < 4) { > zlog_err("%s: socket %d short input expecting nexthop IPv4-addr: > len=%d", > - __func__, zlookup->sock, len); > + __func__, zlookup->sock, length); > return -8; > } > nexthop_tab[num_ifindex].nexthop_addr.s_addr = stream_get_ipv4(s); > - len -= 4; > + length -= 4; > } > else { > nexthop_tab[num_ifindex].nexthop_addr.s_addr = PIM_NET_INADDR_ANY; > @@ -289,7 +286,7 @@ static int zclient_read_nexthop(struct zclient > *zlookup, > return num_ifindex; > } > nexthop_tab[num_ifindex].nexthop_addr.s_addr = stream_get_ipv4(s); > - len -= 4; > + length -= 4; > nexthop_tab[num_ifindex].ifindex = 0; > nexthop_tab[num_ifindex].protocol_distance = distance; > nexthop_tab[num_ifindex].route_metric = metric; > -- > 2.4.2 > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
