On Fri, Mar 22, 2019 at 09:47:51AM +0000, Leon Lagodny wrote:
> Ok, it took a while for switchd to crash again. Here the output of gdb with
> debugging symbols enabled.
>
> (gdb) bt full
> #0 0x00000b62ca3e7a50 in _libc_yp_first (indomain=0xb62cf99f440 "",
> inmap=0xb62eaf445e8 '' <repeats 200 times>..., outkey=0xb62ca3796ff,
> outkeylen=0x0, outval=0xd, outvallen=0x5c0) from /usr/lib/libc.so.92.5
> ysd = (struct dom_binding *) 0xb5f00000001
> r = Variable "r" is not available.
> (gdb) where
> #0 0x00000b62ca3e7a50 in _libc_yp_first (indomain=0xb62cf99f440 "",
> inmap=0xb62eaf445e8 '' <repeats 200 times>..., outkey=0xb62ca3796ff,
> outkeylen=0x0, outval=0xd, outvallen=0x5c0) from /usr/lib/libc.so.92.5
> #1 0x00000b62d5e28350 in ibuf_add (buf=0xb62cf99f440, data=0x0, len=1494) at
> /usr/src/lib/libutil/imsg-buffer.c:97
> #2 0x00000b62d5e26a2e in imsg_add (msg=0xb62cf99f440, data=Variable "data"
> is not available.
> ) at /usr/src/lib/libutil/imsg.c:240
> #3 0x00000b5ffe80ab56 in ofp13_packet_in (sc=0xb62cf99a4d0,
> con=0xb6263883c00, ih=Variable "ih" is not available.
> ) at /usr/src/usr.sbin/switchd/ofp13.c:1150
> #4 0x00000b5ffe80cb32 in ofp13_input (sc=0xb62cf99a4d0, con=0xb6263883c00,
> oh=0xb624549e000, ibuf=0xb626387da80) at /usr/src/usr.sbin/switchd/ofp13.c:621
> #5 0x00000b5ffe809672 in ofp_input (con=0xb6263883c00, ibuf=0xb626387da80)
> at /usr/src/usr.sbin/switchd/ofp.c:159
> #6 0x00000b5ffe811625 in ofrelay_input_done (con=0xb6263883c00,
> buf=0xb626387da80) at /usr/src/usr.sbin/switchd/ofrelay.c:282
> #7 0x00000b5ffe811157 in ofrelay_event (fd=8, event=2, arg=0xb6263883c00) at
> /usr/src/usr.sbin/switchd/ofrelay.c:138
> #8 0x00000b62394a7bbd in event_base_loop (base=0xb6263883400, flags=0) at
> /usr/src/lib/libevent/event.c:350
> #9 0x00000b5ffe803a98 in proc_run (ps=0xb62cf99a4d0, p=0xb5ffea22000,
> procs=0xb5ffea22180, nproc=2, run=0xb5ffe809360 <ofp_run>, arg=0x0) at
> /usr/src/usr.sbin/switchd/proc.c:602
> #10 0x00000b5ffe802a8b in proc_init (ps=Variable "ps" is not available.
> ) at /usr/src/usr.sbin/switchd/proc.c:260
> #11 0x00000b5ffe807660 in main (argc=5, argv=0x7f7ffffdd658) at
> /usr/src/usr.sbin/switchd/switchd.c:187
Hi,
This looks like a case where the switch requires switchd to supply a copy
of the packet being forwarded, but the latter doesn't set its buffer for
the packet data. Currently, this can happen if the copy of the packet
recieved from the switch is too short (source/destination pairs can't be
recovered), is non-unicast, or when switchd has to fall back to flooding
traffic.
This diff, based off of one by guenther, factors out the check for short
packets, stopping before forwarding decisions are made if the full packet
is needed by the switch. It also sets the packet buffer early for cases
where it is needed otherwise. This should prevent the above situation.
Not sure if it's excess noise, but I've also turned a few bzero's into
memset's along the way.
I've been running switchd with this diff for ~2 days without issue.
OK?
Thanks,
Ayaka
Index: switchd/ofp10.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/ofp10.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 ofp10.c
--- switchd/ofp10.c 9 Sep 2018 14:21:32 -0000 1.20
+++ switchd/ofp10.c 30 Apr 2019 06:28:52 -0000
@@ -342,7 +342,7 @@ ofp10_packet_match(struct packet *pkt, s
{
struct ether_header *eh = pkt->pkt_eh;
- bzero(m, sizeof(*m));
+ memset(m, 0, sizeof(*m));
m->m_wildcards = htonl(~flags);
if ((flags & (OFP10_WILDCARD_DL_SRC|OFP10_WILDCARD_DL_DST)) &&
@@ -377,12 +377,17 @@ ofp10_packet_in(struct switchd *sc, stru
if ((pin = ibuf_getdata(ibuf, sizeof(*pin))) == NULL)
return (-1);
- bzero(&pkt, sizeof(pkt));
+ memset(&pkt, 0, sizeof(pkt));
len = ntohs(pin->pin_total_len);
+
srcport = ntohs(pin->pin_port);
+ if (packet_ether_input(ibuf, len, &pkt) == -1 &&
+ pin->pin_buffer_id == htonl(OFP_PKTOUT_NO_BUFFER))
+ return(-1);
+
if (packet_input(sc, con->con_switch,
- srcport, &dstport, ibuf, len, &pkt) == -1 ||
+ srcport, &dstport, &pkt) == -1 ||
(dstport > OFP10_PORT_MAX &&
dstport != OFP10_PORT_LOCAL &&
dstport != OFP10_PORT_CONTROLLER)) {
Index: switchd/ofp13.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.44
diff -u -p -u -r1.44 ofp13.c
--- switchd/ofp13.c 9 Sep 2018 14:21:32 -0000 1.44
+++ switchd/ofp13.c 30 Apr 2019 06:28:52 -0000
@@ -1029,7 +1029,7 @@ ofp13_packet_in(struct switchd *sc, stru
if (pin->pin_reason != OFP_PKTIN_REASON_NO_MATCH)
return (-1);
- bzero(&pkt, sizeof(pkt));
+ memset(&pkt, 0, sizeof(pkt));
len = ntohs(pin->pin_total_len);
/* very basic way of getting the source port */
@@ -1070,8 +1070,12 @@ ofp13_packet_in(struct switchd *sc, stru
if (ibuf_getdata(ibuf, off) == NULL)
return (-1);
+ if (packet_ether_input(ibuf, len, &pkt) == -1 &&
+ pin->pin_buffer_id == htonl(OFP_PKTOUT_NO_BUFFER))
+ return(-1);
+
if (packet_input(sc, con->con_switch,
- srcport, &dstport, ibuf, len, &pkt) == -1 ||
+ srcport, &dstport, &pkt) == -1 ||
(dstport > OFP_PORT_MAX &&
dstport != OFP_PORT_LOCAL &&
dstport != OFP_PORT_CONTROLLER)) {
Index: switchd/packet.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/packet.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 packet.c
--- switchd/packet.c 6 Aug 2017 17:31:19 -0000 1.5
+++ switchd/packet.c 30 Apr 2019 06:28:52 -0000
@@ -50,14 +50,10 @@ packet_ether_unicast(uint8_t *ea)
}
int
-packet_input(struct switchd *sc, struct switch_control *sw, uint32_t srcport,
- uint32_t *dstport, struct ibuf *ibuf, size_t len, struct packet *pkt)
+packet_ether_input(struct ibuf *ibuf, size_t len, struct packet *pkt)
{
struct ether_header *eh;
- struct macaddr *src, *dst;
- if (sw == NULL)
- return (-1);
if (len < sizeof(*eh))
return (-1);
@@ -66,8 +62,24 @@ packet_input(struct switchd *sc, struct
log_debug("short packet");
return (-1);
}
- len -= sizeof(*eh);
+ pkt->pkt_eh = eh;
+ pkt->pkt_buf = (uint8_t *)eh;
+
+ return (0);
+}
+
+int
+packet_input(struct switchd *sc, struct switch_control *sw, uint32_t srcport,
+ uint32_t *dstport, struct packet *pkt)
+{
+ struct ether_header *eh;
+ struct macaddr *src, *dst;
+
+ if (sw == NULL)
+ return (-1);
+
+ eh = pkt->pkt_eh;
if ((packet_ether_unicast(eh->ether_shost) == -1) ||
(src = switch_learn(sc, sw, eh->ether_shost, srcport)) == NULL)
return (-1);
@@ -85,9 +97,6 @@ packet_input(struct switchd *sc, struct
if (dstport)
*dstport = dst == NULL ? OFP_PORT_ANY : dst->mac_port;
-
- pkt->pkt_eh = eh;
- pkt->pkt_buf = (uint8_t *)eh;
return (0);
}
Index: switchd/switchd.h
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/switchd.h,v
retrieving revision 1.28
diff -u -p -u -r1.28 switchd.h
--- switchd/switchd.h 22 Dec 2016 15:31:43 -0000 1.28
+++ switchd/switchd.h 30 Apr 2019 06:28:52 -0000
@@ -211,9 +211,9 @@ struct switch_connection *
switchd_connbyaddr(struct switchd *, struct sockaddr *);
/* packet.c */
+int packet_ether_input(struct ibuf *, size_t, struct packet *);
int packet_input(struct switchd *, struct switch_control *,
- uint32_t, uint32_t *, struct ibuf *, size_t,
- struct packet *);
+ uint32_t, uint32_t *, struct packet *);
/* switch.c */
void switch_init(struct switchd *);
