From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6EA91C43381 for ; Thu, 28 Mar 2019 00:47:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3C873206BA for ; Thu, 28 Mar 2019 00:47:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mGYOIacC" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727837AbfC1Ar2 (ORCPT ); Wed, 27 Mar 2019 20:47:28 -0400 Received: from mail-it1-f195.google.com ([209.85.166.195]:32782 "EHLO mail-it1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726102AbfC1Ar2 (ORCPT ); Wed, 27 Mar 2019 20:47:28 -0400 Received: by mail-it1-f195.google.com with SMTP id v8so6968351itf.0 for ; Wed, 27 Mar 2019 17:47:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=nh5Dtv2e7kSjha8EY3xhxYBXxs/VFvuH1a0YHAOo26w=; b=mGYOIacC5NAI4YKUwNjPlKELg4c5oCt+rljB947BoPJzHmkvscjfsrMTe+DfQfyhbk oYuAXPzF73EiovRxBY4NC62CnAIzEin2UicHuN9cNJIZdNjjkPl9RJNpOoXz5KvSrnOQ UUjLgLoZsj3AXDp6AvsLSRPOIyN2IIpkgc7j2We+sxQ5B70aCoukBvrN5Yu4vycfkEuH NTwEeEbChpX/AEYwzHrFpqJkcsxL8UGEUlnWM3Jv3c7Y+0Fkce+eYhr3cNSsDiWW7PqF fBQicxN/LXqOPtl39gyTJBClaKSfQTdIg1x8YmspmpXgTC0jjVZQJ30BvuFbdc5IKRdC 1Ehw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=nh5Dtv2e7kSjha8EY3xhxYBXxs/VFvuH1a0YHAOo26w=; b=JscQKPA+nB2XvCbA2hjQuc9WnJ4NyEhd3Sic6KD1ecEOGqKKXeEcUGiMUxa8aIKE1q wlO6YZVTqF97OpBbWxhQR+kCySSCqiywE2ZYkCxwPiX6xwn4oJRoyEXQHWa9I5VxbP5p uVgy5WaV4tw2e4iQgOpPGPQxdHzDv732mc31/0LPVL1YRx5wipTnvtU75//N2ePcG2B3 yWoW6MV5ljiohYZRoNiDQWvIowgzMUM9zDUdexfw/6WQIXVHcgkHGhU8dTcYlmLacXkK YiObsIZhpifXEEUY2CBhbXZDmey2BdLCWDl6gskOpDv7X7hznlz7LWsqKqGks0bxYnoC rfVw== X-Gm-Message-State: APjAAAVUb0AjQTzVRiSE3yDG6MOwTME9hKJMuqqPq0FR/2oNitXonrov PvYHJ81C20ce2BmTArhlb7Y= X-Google-Smtp-Source: APXvYqy3Nu/0Kd0G1fqlhC1hICrdzSWrwbQTE+ZaVvyZgYbwca0wCBLlHGSn6B1JBqkjZYEKXbbO3g== X-Received: by 2002:a02:ad05:: with SMTP id s5mr18971300jan.71.1553734047794; Wed, 27 Mar 2019 17:47:27 -0700 (PDT) Received: from ?IPv6:2601:284:8200:5cfb:4430:3868:1dbd:b2ac? ([2601:284:8200:5cfb:4430:3868:1dbd:b2ac]) by smtp.googlemail.com with ESMTPSA id f8sm1504368itc.27.2019.03.27.17.47.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Mar 2019 17:47:26 -0700 (PDT) Subject: Re: [PATCH v2 net-next 05/13] ipv6: Create init helper for fib6_nh To: Alexei Starovoitov , David Ahern Cc: davem@davemloft.net, netdev@vger.kernel.org, idosch@mellanox.com, jiri@mellanox.com, saeedm@mellanox.com References: <20190327182329.18149-1-dsahern@kernel.org> <20190327182329.18149-6-dsahern@kernel.org> <20190327225210.kbpivej33a72adwq@ast-mbp> From: David Ahern Message-ID: <8e0e2afc-c8e7-8728-7b4d-f9c64987d46a@gmail.com> Date: Wed, 27 Mar 2019 18:47:23 -0600 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190327225210.kbpivej33a72adwq@ast-mbp> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 3/27/19 4:52 PM, Alexei Starovoitov wrote: >> + >> + /* We cannot add true routes via loopback here, >> + * they would result in kernel looping; promote them to reject routes >> + */ >> + addr_type = ipv6_addr_type(&cfg->fc_dst); >> + if ((cfg->fc_flags & RTF_REJECT) || >> + (dev && (dev->flags & IFF_LOOPBACK) && >> + !(addr_type & IPV6_ADDR_LOOPBACK) && >> + !(cfg->fc_flags & RTF_LOCAL))) { >> + /* hold loopback dev/idev if we haven't done so. */ >> + if (dev != net->loopback_dev) { >> + if (dev) { >> + dev_put(dev); >> + in6_dev_put(idev); >> + } >> + dev = net->loopback_dev; >> + dev_hold(dev); >> + idev = in6_dev_get(dev); >> + if (!idev) { >> + err = -ENODEV; >> + goto out; >> + } >> + } >> + cfg->fc_flags = RTF_REJECT | RTF_NONEXTHOP; > > imo it would be cleaner not to mess with cfg. > Ideally it should be marked 'const'. Existing code sets those flags but on a fib6_info. This is not used for nexthop objects and is kept here to not duplicate this if branch in the create_info that uses it. This check affects both which device is used as well as the flags. > >> + goto set_dev; >> + } >> + >> + if (cfg->fc_flags & RTF_GATEWAY) { >> + err = ip6_validate_gw(net, cfg, &dev, &idev, extack); >> + if (err) >> + goto out; >> + >> + fib6_nh->nh_gw = cfg->fc_gateway; >> + } >> + >> + err = -ENODEV; >> + if (!dev) >> + goto out; >> + >> + if (idev->cnf.disable_ipv6) { >> + NL_SET_ERR_MSG(extack, "IPv6 is disabled on nexthop device"); >> + err = -EACCES; >> + goto out; >> + } >> + >> + if (!(dev->flags & IFF_UP) && !cfg->fc_ignore_dev_down) { >> + NL_SET_ERR_MSG(extack, "Nexthop device is not up"); >> + err = -ENETDOWN; >> + goto out; >> + } >> + >> + if (!(cfg->fc_flags & (RTF_LOCAL | RTF_ANYCAST)) && >> + !netif_carrier_ok(dev)) >> + fib6_nh->nh_flags |= RTNH_F_LINKDOWN; >> + >> +set_dev: >> + fib6_nh->nh_dev = dev; >> + err = 0; >> +out: >> + if (idev) >> + in6_dev_put(idev); >> + >> + if (err) { >> + lwtstate_put(fib6_nh->nh_lwtstate); > > lwtstate_put() is missing in the error path of existing code. > Is this a bug fix? > Why there is nothing about this in commit log? Existing code has a different cleanup path. This is done explicitly here per a request from Ido in v1 that the new function be symmetric in its cleanup on an error.