Changeset View
Changeset View
Standalone View
Standalone View
sys/net/route/nhop_ctl.c
| Show First 20 Lines • Show All 78 Lines • ▼ Show 20 Lines | |||||
| static int dump_nhop_entry(struct rib_head *rh, struct nhop_object *nh, struct sysctl_req *w); | static int dump_nhop_entry(struct rib_head *rh, struct nhop_object *nh, struct sysctl_req *w); | ||||
| static struct nhop_priv *alloc_nhop_structure(void); | static struct nhop_priv *alloc_nhop_structure(void); | ||||
| static int get_nhop(struct rib_head *rnh, struct rt_addrinfo *info, | static int get_nhop(struct rib_head *rnh, struct rt_addrinfo *info, | ||||
| struct nhop_priv **pnh_priv); | struct nhop_priv **pnh_priv); | ||||
| static int finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info, | static int finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info, | ||||
| struct nhop_priv *nh_priv); | struct nhop_priv *nh_priv); | ||||
| static struct ifnet *get_aifp(const struct nhop_object *nh, int reference); | static struct ifnet *get_aifp(const struct nhop_object *nh); | ||||
| static void fill_sdl_from_ifp(struct sockaddr_dl_short *sdl, const struct ifnet *ifp); | static void fill_sdl_from_ifp(struct sockaddr_dl_short *sdl, const struct ifnet *ifp); | ||||
| static void destroy_nhop_epoch(epoch_context_t ctx); | static void destroy_nhop_epoch(epoch_context_t ctx); | ||||
| static void destroy_nhop(struct nhop_priv *nh_priv); | static void destroy_nhop(struct nhop_priv *nh_priv); | ||||
| static void print_nhop(const char *prefix, const struct nhop_object *nh); | static void print_nhop(const char *prefix, const struct nhop_object *nh); | ||||
| _Static_assert(__offsetof(struct nhop_object, nh_ifp) == 32, | _Static_assert(__offsetof(struct nhop_object, nh_ifp) == 32, | ||||
| Show All 19 Lines | |||||
| /* | /* | ||||
| * Fetches the interface of source address used by the route. | * Fetches the interface of source address used by the route. | ||||
| * In all cases except interface-address-route it would be the | * In all cases except interface-address-route it would be the | ||||
| * same as the transmit interfaces. | * same as the transmit interfaces. | ||||
| * However, for the interface address this function will return | * However, for the interface address this function will return | ||||
| * this interface ifp instead of loopback. This is needed to support | * this interface ifp instead of loopback. This is needed to support | ||||
| * link-local IPv6 loopback communications. | * link-local IPv6 loopback communications. | ||||
| * | * | ||||
| * If @reference is non-zero, found ifp is referenced. | |||||
| * | |||||
| * Returns found ifp. | * Returns found ifp. | ||||
| */ | */ | ||||
| static struct ifnet * | static struct ifnet * | ||||
| get_aifp(const struct nhop_object *nh, int reference) | get_aifp(const struct nhop_object *nh) | ||||
| { | { | ||||
| struct ifnet *aifp = NULL; | struct ifnet *aifp = NULL; | ||||
| /* | /* | ||||
| * Adjust the "outgoing" interface. If we're going to loop | * Adjust the "outgoing" interface. If we're going to loop | ||||
| * the packet back to ourselves, the ifp would be the loopback | * the packet back to ourselves, the ifp would be the loopback | ||||
| * interface. However, we'd rather know the interface associated | * interface. However, we'd rather know the interface associated | ||||
| * to the destination address (which should probably be one of | * to the destination address (which should probably be one of | ||||
| * our own addresses). | * our own addresses). | ||||
| */ | */ | ||||
| if ((nh->nh_ifp->if_flags & IFF_LOOPBACK) && | if ((nh->nh_ifp->if_flags & IFF_LOOPBACK) && | ||||
| nh->gw_sa.sa_family == AF_LINK) { | nh->gw_sa.sa_family == AF_LINK) { | ||||
| if (reference) | |||||
| aifp = ifnet_byindex_ref(nh->gwl_sa.sdl_index); | |||||
| else | |||||
| aifp = ifnet_byindex(nh->gwl_sa.sdl_index); | aifp = ifnet_byindex(nh->gwl_sa.sdl_index); | ||||
| if (aifp == NULL) { | if (aifp == NULL) { | ||||
| DPRINTF("unable to get aifp for %s index %d", | DPRINTF("unable to get aifp for %s index %d", | ||||
| if_name(nh->nh_ifp), nh->gwl_sa.sdl_index); | if_name(nh->nh_ifp), nh->gwl_sa.sdl_index); | ||||
| } | } | ||||
| } | } | ||||
| if (aifp == NULL) { | if (aifp == NULL) | ||||
| aifp = nh->nh_ifp; | aifp = nh->nh_ifp; | ||||
| if (reference) | |||||
| if_ref(aifp); | |||||
| } | |||||
| return (aifp); | return (aifp); | ||||
| } | } | ||||
| int | int | ||||
| cmp_priv(const struct nhop_priv *_one, const struct nhop_priv *_two) | cmp_priv(const struct nhop_priv *_one, const struct nhop_priv *_two) | ||||
| { | { | ||||
| ▲ Show 20 Lines • Show All 128 Lines • ▼ Show 20 Lines | fill_nhop_from_info(struct nhop_priv *nh_priv, struct rt_addrinfo *info) | ||||
| nh->nh_flags = convert_rt_to_nh_flags(rt_flags); | nh->nh_flags = convert_rt_to_nh_flags(rt_flags); | ||||
| set_nhop_mtu_from_info(nh, info); | set_nhop_mtu_from_info(nh, info); | ||||
| if ((error = set_nhop_gw_from_info(nh, info)) != 0) | if ((error = set_nhop_gw_from_info(nh, info)) != 0) | ||||
| return (error); | return (error); | ||||
| nh->nh_ifp = info->rti_ifa->ifa_ifp; | nh->nh_ifp = info->rti_ifa->ifa_ifp; | ||||
| nh->nh_ifa = info->rti_ifa; | nh->nh_ifa = info->rti_ifa; | ||||
| /* depends on the gateway */ | /* depends on the gateway */ | ||||
| nh->nh_aifp = get_aifp(nh, 0); | nh->nh_aifp = get_aifp(nh); | ||||
| /* | /* | ||||
| * Note some of the remaining data is set by the | * Note some of the remaining data is set by the | ||||
| * per-address-family pre-add hook. | * per-address-family pre-add hook. | ||||
| */ | */ | ||||
| return (0); | return (0); | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 124 Lines • ▼ Show 20 Lines | alter_nhop_from_info(struct nhop_object *nh, struct rt_addrinfo *info) | ||||
| } | } | ||||
| /* Update datapath flags */ | /* Update datapath flags */ | ||||
| nh->nh_flags = convert_rt_to_nh_flags(nh->nh_priv->rt_flags); | nh->nh_flags = convert_rt_to_nh_flags(nh->nh_priv->rt_flags); | ||||
| if (info->rti_ifa != NULL) | if (info->rti_ifa != NULL) | ||||
| nh->nh_ifa = info->rti_ifa; | nh->nh_ifa = info->rti_ifa; | ||||
| if (info->rti_ifp != NULL) | if (info->rti_ifp != NULL) | ||||
| nh->nh_ifp = info->rti_ifp; | nh->nh_ifp = info->rti_ifp; | ||||
| nh->nh_aifp = get_aifp(nh, 0); | nh->nh_aifp = get_aifp(nh); | ||||
| return (0); | return (0); | ||||
| } | } | ||||
| /* | /* | ||||
| * Creates new nexthop based on @nh_orig and augmentation data from @info. | * Creates new nexthop based on @nh_orig and augmentation data from @info. | ||||
| * Helper function used in the route changes, please see | * Helper function used in the route changes, please see | ||||
| * alter_nhop_from_info() comments for more details. | * alter_nhop_from_info() comments for more details. | ||||
| ▲ Show 20 Lines • Show All 57 Lines • ▼ Show 20 Lines | alloc_nhop_structure() | ||||
| nh_priv = (struct nhop_priv *)((char *)nh + NHOP_OBJECT_ALIGNED_SIZE); | nh_priv = (struct nhop_priv *)((char *)nh + NHOP_OBJECT_ALIGNED_SIZE); | ||||
| nh->nh_priv = nh_priv; | nh->nh_priv = nh_priv; | ||||
| nh_priv->nh = nh; | nh_priv->nh = nh; | ||||
| return (nh_priv); | return (nh_priv); | ||||
| } | } | ||||
| static bool | |||||
| reference_nhop_deps(struct nhop_object *nh) | |||||
| { | |||||
| if (!ifa_try_ref(nh->nh_ifa)) | |||||
| return (false); | |||||
| nh->nh_aifp = get_aifp(nh); | |||||
| if (!if_try_ref(nh->nh_aifp)) { | |||||
| ifa_free(nh->nh_ifa); | |||||
| return (false); | |||||
| } | |||||
| DPRINTF("AIFP: %p nh_ifp %p", nh->nh_aifp, nh->nh_ifp); | |||||
| if (!if_try_ref(nh->nh_ifp)) { | |||||
| ifa_free(nh->nh_ifa); | |||||
| if_rele(nh->nh_aifp); | |||||
| return (false); | |||||
| } | |||||
| return (true); | |||||
| } | |||||
| /* | /* | ||||
| * Alocates/references the remaining bits of nexthop data and links | * Alocates/references the remaining bits of nexthop data and links | ||||
| * it to the hash table. | * it to the hash table. | ||||
| * Returns 0 if successful, | * Returns 0 if successful, | ||||
| * errno otherwise. @nh_priv is freed in case of error. | * errno otherwise. @nh_priv is freed in case of error. | ||||
| */ | */ | ||||
| static int | static int | ||||
| finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info, | finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info, | ||||
| struct nhop_priv *nh_priv) | struct nhop_priv *nh_priv) | ||||
| { | { | ||||
| struct nhop_object *nh; | struct nhop_object *nh = nh_priv->nh; | ||||
| nh = nh_priv->nh; | |||||
| /* Allocate per-cpu packet counter */ | /* Allocate per-cpu packet counter */ | ||||
| nh->nh_pksent = counter_u64_alloc(M_NOWAIT); | nh->nh_pksent = counter_u64_alloc(M_NOWAIT); | ||||
| if (nh->nh_pksent == NULL) { | if (nh->nh_pksent == NULL) { | ||||
| uma_zfree(nhops_zone, nh); | uma_zfree(nhops_zone, nh); | ||||
| RTSTAT_INC(rts_nh_alloc_failure); | RTSTAT_INC(rts_nh_alloc_failure); | ||||
| DPRINTF("nh_alloc_finalize failed"); | DPRINTF("nh_alloc_finalize failed"); | ||||
| return (ENOMEM); | return (ENOMEM); | ||||
| } | } | ||||
| if (!reference_nhop_deps(nh)) { | |||||
| counter_u64_free(nh->nh_pksent); | |||||
| uma_zfree(nhops_zone, nh); | |||||
| RTSTAT_INC(rts_nh_alloc_failure); | |||||
| DPRINTF("nh_alloc_finalize failed - reference failure"); | |||||
| return (EAGAIN); | |||||
| } | |||||
| /* Save vnet to ease destruction */ | /* Save vnet to ease destruction */ | ||||
donner: Can you merge that into a single
if (ng->...sent == NULL || !ref...deps(ng)) {
if (ng… | |||||
Done Inline ActionsYeah. Tried that, but it doesn't look any better, at least in my view. There are only 2 lines that is actually the same in these blocks. Merging two blocks into one adds too many conditions (how do we easily distinguish between ENOMEM and EAGAIN?). I'd rather leave them as is - at least it's easy to read. melifaro: Yeah. Tried that, but it doesn't look any better, at least in my view.
There are only 2 lines… | |||||
| nh_priv->nh_vnet = curvnet; | nh_priv->nh_vnet = curvnet; | ||||
| /* Reference external objects and calculate (referenced) ifa */ | |||||
| if_ref(nh->nh_ifp); | |||||
| ifa_ref(nh->nh_ifa); | |||||
| nh->nh_aifp = get_aifp(nh, 1); | |||||
| DPRINTF("AIFP: %p nh_ifp %p", nh->nh_aifp, nh->nh_ifp); | |||||
| refcount_init(&nh_priv->nh_refcnt, 1); | refcount_init(&nh_priv->nh_refcnt, 1); | ||||
| /* Please see nhop_free() comments on the initial value */ | /* Please see nhop_free() comments on the initial value */ | ||||
| refcount_init(&nh_priv->nh_linked, 2); | refcount_init(&nh_priv->nh_linked, 2); | ||||
| print_nhop("FINALIZE", nh); | print_nhop("FINALIZE", nh); | ||||
| ▲ Show 20 Lines • Show All 360 Lines • Show Last 20 Lines | |||||
Can you merge that into a single
if (ng->...sent == NULL || !ref...deps(ng)) { if (ng->...send) counter..free();to avoid code duplication?
Or something like
if (!ng->...sent) { error = ENOMEN; goto fail; }