From owner-svn-src-all@freebsd.org Wed Aug 8 16:40:54 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D4AC81061CE2; Wed, 8 Aug 2018 16:40:53 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 27DBC853C9; Wed, 8 Aug 2018 16:40:52 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (localhost [127.0.0.1]) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3) with ESMTP id w78GegxI012856; Wed, 8 Aug 2018 09:40:43 -0700 (PDT) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: (from freebsd@localhost) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3/Submit) id w78Gegho012855; Wed, 8 Aug 2018 09:40:42 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201808081640.w78Gegho012855@pdx.rh.CN85.dnsmgr.net> Subject: Re: svn commit: r337462 - in stable/11/sys: net netinet netinet6 In-Reply-To: <201808081617.w78GHoch061371@repo.freebsd.org> To: "Andrey V. Elsukov" Date: Wed, 8 Aug 2018 09:40:42 -0700 (PDT) CC: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Aug 2018 16:40:54 -0000 > Author: ae > Date: Wed Aug 8 16:17:50 2018 > New Revision: 337462 > URL: https://svnweb.freebsd.org/changeset/base/337462 > > Log: > MFC r331098 (by melifaro): > Fix outgoing TCP/UDP packet drop on arp/ndp entry expiration. > > Current arp/nd code relies on the feedback from the datapath indicating > that the entry is still used. This mechanism is incorporated into the > arpresolve()/nd6_resolve() routines. After the inpcb route cache > introduction, the packet path for the locally-originated packets changed, > passing cached lle pointer to the ether_output() directly. This resulted > in the arp/ndp entry expire each time exactly after the configured max_age > interval. During the small window between the ARP/NDP request and reply > from the router, most of the packets got lost. > > Fix this behaviour by plugging datapath notification code to the packet > path used by route cache. Unify the notification code by using single > inlined function with the per-AF callbacks. Is this worthy of an EN for 11.2? This is a rather annoying problem for some people. > Modified: > stable/11/sys/net/if_ethersubr.c > stable/11/sys/net/if_llatbl.h > stable/11/sys/netinet/if_ether.c > stable/11/sys/netinet/in.c > stable/11/sys/netinet6/in6.c > Directory Properties: > stable/11/ (props changed) > > Modified: stable/11/sys/net/if_ethersubr.c > ============================================================================== > --- stable/11/sys/net/if_ethersubr.c Wed Aug 8 16:11:46 2018 (r337461) > +++ stable/11/sys/net/if_ethersubr.c Wed Aug 8 16:17:50 2018 (r337462) > @@ -313,7 +313,13 @@ ether_output(struct ifnet *ifp, struct mbuf *m, > if (lle == NULL) { > /* if we lookup, keep cache */ > addref = 1; > - } > + } else > + /* > + * Notify LLE code that > + * the entry was used > + * by datapath. > + */ > + llentry_mark_used(lle); > } > if (lle != NULL) { > phdr = lle->r_linkdata; > > Modified: stable/11/sys/net/if_llatbl.h > ============================================================================== > --- stable/11/sys/net/if_llatbl.h Wed Aug 8 16:11:46 2018 (r337461) > +++ stable/11/sys/net/if_llatbl.h Wed Aug 8 16:17:50 2018 (r337462) > @@ -155,6 +155,7 @@ typedef void (llt_fill_sa_entry_t)(const struct llentr > typedef void (llt_free_tbl_t)(struct lltable *); > typedef void (llt_link_entry_t)(struct lltable *, struct llentry *); > typedef void (llt_unlink_entry_t)(struct llentry *); > +typedef void (llt_mark_used_t)(struct llentry *); > > typedef int (llt_foreach_cb_t)(struct lltable *, struct llentry *, void *); > typedef int (llt_foreach_entry_t)(struct lltable *, llt_foreach_cb_t *, void *); > @@ -179,6 +180,7 @@ struct lltable { > llt_unlink_entry_t *llt_unlink_entry; > llt_fill_sa_entry_t *llt_fill_sa_entry; > llt_free_tbl_t *llt_free_tbl; > + llt_mark_used_t *llt_mark_used; > }; > > MALLOC_DECLARE(M_LLTABLE); > @@ -251,6 +253,19 @@ lla_lookup(struct lltable *llt, u_int flags, const str > { > > return (llt->llt_lookup(llt, flags, l3addr)); > +} > + > +/* > + * Notify the LLE code that the entry was used by datapath. > + */ > +static __inline void > +llentry_mark_used(struct llentry *lle) > +{ > + > + if (lle->r_skip_req == 0) > + return; > + if ((lle->r_flags & RLLE_VALID) != 0) > + lle->lle_tbl->llt_mark_used(lle); > } > > int lla_rt_output(struct rt_msghdr *, struct rt_addrinfo *); > > Modified: stable/11/sys/netinet/if_ether.c > ============================================================================== > --- stable/11/sys/netinet/if_ether.c Wed Aug 8 16:11:46 2018 (r337461) > +++ stable/11/sys/netinet/if_ether.c Wed Aug 8 16:17:50 2018 (r337462) > @@ -502,12 +502,8 @@ arpresolve_full(struct ifnet *ifp, int is_gw, int flag > } > bcopy(lladdr, desten, ll_len); > > - /* Check if we have feedback request from arptimer() */ > - if (la->r_skip_req != 0) { > - LLE_REQ_LOCK(la); > - la->r_skip_req = 0; /* Notify that entry was used */ > - LLE_REQ_UNLOCK(la); > - } > + /* Notify LLE code that the entry was used by datapath */ > + llentry_mark_used(la); > if (pflags != NULL) > *pflags = la->la_flags & (LLE_VALID|LLE_IFADDR); > if (plle) { > @@ -638,12 +634,8 @@ arpresolve(struct ifnet *ifp, int is_gw, struct mbuf * > bcopy(la->r_linkdata, desten, la->r_hdrlen); > if (pflags != NULL) > *pflags = LLE_VALID | (la->r_flags & RLLE_IFADDR); > - /* Check if we have feedback request from arptimer() */ > - if (la->r_skip_req != 0) { > - LLE_REQ_LOCK(la); > - la->r_skip_req = 0; /* Notify that entry was used */ > - LLE_REQ_UNLOCK(la); > - } > + /* Notify the LLE handling code that the entry was used. */ > + llentry_mark_used(la); > if (plle) { > LLE_ADDREF(la); > *plle = la; > > Modified: stable/11/sys/netinet/in.c > ============================================================================== > --- stable/11/sys/netinet/in.c Wed Aug 8 16:11:46 2018 (r337461) > +++ stable/11/sys/netinet/in.c Wed Aug 8 16:17:50 2018 (r337462) > @@ -1056,6 +1056,19 @@ in_lltable_destroy_lle_unlocked(struct llentry *lle) > } > > /* > + * Called by the datapath to indicate that > + * the entry was used. > + */ > +static void > +in_lltable_mark_used(struct llentry *lle) > +{ > + > + LLE_REQ_LOCK(lle); > + lle->r_skip_req = 0; > + LLE_REQ_UNLOCK(lle); > +} > + > +/* > * Called by LLE_FREE_LOCKED when number of references > * drops to zero. > */ > @@ -1454,6 +1467,7 @@ in_lltattach(struct ifnet *ifp) > llt->llt_fill_sa_entry = in_lltable_fill_sa_entry; > llt->llt_free_entry = in_lltable_free_entry; > llt->llt_match_prefix = in_lltable_match_prefix; > + llt->llt_mark_used = in_lltable_mark_used; > lltable_link(llt); > > return (llt); > > Modified: stable/11/sys/netinet6/in6.c > ============================================================================== > --- stable/11/sys/netinet6/in6.c Wed Aug 8 16:11:46 2018 (r337461) > +++ stable/11/sys/netinet6/in6.c Wed Aug 8 16:17:50 2018 (r337462) > @@ -2145,6 +2145,25 @@ in6_lltable_rtcheck(struct ifnet *ifp, > return 0; > } > > +/* > + * Called by the datapath to indicate that the entry was used. > + */ > +static void > +in6_lltable_mark_used(struct llentry *lle) > +{ > + > + LLE_REQ_LOCK(lle); > + lle->r_skip_req = 0; > + > + /* > + * Set the hit time so the callback function > + * can determine the remaining time before > + * transiting to the DELAY state. > + */ > + lle->lle_hittime = time_uptime; > + LLE_REQ_UNLOCK(lle); > +} > + > static inline uint32_t > in6_lltable_hash_dst(const struct in6_addr *dst, uint32_t hsize) > { > @@ -2377,6 +2396,7 @@ in6_lltattach(struct ifnet *ifp) > llt->llt_fill_sa_entry = in6_lltable_fill_sa_entry; > llt->llt_free_entry = in6_lltable_free_entry; > llt->llt_match_prefix = in6_lltable_match_prefix; > + llt->llt_mark_used = in6_lltable_mark_used; > lltable_link(llt); > > return (llt); > > -- Rod Grimes rgrimes@freebsd.org