* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: Cong Wang @ 2019-07-02 3:12 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Miller, Linux Kernel Network Developers, Davide Caratti,
Chris Mi
In-Reply-To: <20190702023730.GA1729@bombadil.infradead.org>
On Mon, Jul 1, 2019 at 7:37 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
> > From: Cong Wang <xiyou.wangcong@gmail.com>
> > Date: Fri, 28 Jun 2019 11:03:40 -0700
> >
> > > idr_get_next_ul() is problematic by design, it can't handle
> > > the following overflow case well on 32-bit CPU:
> > >
> > > u32 id = UINT_MAX;
> > > idr_alloc_u32(&id);
> > > while (idr_get_next_ul(&id) != NULL)
> > > id++;
> > >
> > > when 'id' overflows and becomes 0 after UINT_MAX, the loop
> > > goes infinite.
> > >
> > > Fix this by eliminating external users of idr_get_next_ul()
> > > and migrating them to idr_for_each_entry_continue_ul(). And
> > > add an additional parameter for these iteration macros to detect
> > > overflow properly.
> > >
> > > Please merge this through networking tree, as all the users
> > > are in networking subsystem.
> >
> > Series applied, thanks Cong.
>
> Ugh, I don't even get the weekend to reply?
>
> I think this is just a bad idea. It'd be better to apply the conversion
> patches to use XArray than fix up this crappy interface. I didn't
> reply before because I wanted to check those patches still apply and
> post them as part of the response. Now they're definitely broken and
> need to be redone.
You can always do refactoring for net-next/linux-next. It is never late
for it.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC
From: Jakub Kicinski @ 2019-07-02 2:47 UTC (permalink / raw)
To: Catherine Sullivan
Cc: netdev, Sagi Shahar, Jon Olson, Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190701225755.209250-2-csully@google.com>
On Mon, 1 Jul 2019 15:57:52 -0700, Catherine Sullivan wrote:
> Add a driver framework for the Compute Engine Virtual NIC that will be
> available in the future.
>
> At this point the only functionality is loading the driver.
>
> Signed-off-by: Catherine Sullivan <csully@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Jon Olson <jonolson@google.com>
> Acked-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Luigi Rizzo <lrizzo@google.com>
> +Admin Queue (AQ)
> +----------------
> +The Admin Queue is a PAGE_SIZE memory block, treated as an array of AQ
> +commands, used by the driver to issue commands to the device and set up
> +resources.The driver and the device maintain a count of how many commands
> +have been submitted and executed. To issue AQ commands, the driver must do
> +the following (with proper locking):
> +
> +1) Copy new commands into next available slots in the AQ array
> +2) Increment its counter by he number of new commands
s/he/the/
> +3) Write the counter into the GVE_ADMIN_QUEUE_DOORBELL register
> +4) Poll the ADMIN_QUEUE_EVENT_COUNTER register until it equals
> + the value written to the doorbell, or until a timeout.
> +
> +The device will update the status field in each AQ command reported as
> +executed through the ADMIN_QUEUE_EVENT_COUNTER register.
> +
> +/* This function is not threadsafe - the caller is responsible for any
> + * necessary locks.
> + */
> +int gve_adminq_execute_cmd(struct gve_priv *priv,
> + union gve_adminq_command *cmd_orig)
> +{
> + union gve_adminq_command *cmd;
> + u32 status = 0;
> + u32 prod_cnt;
> +
> + cmd = &priv->adminq[priv->adminq_prod_cnt & priv->adminq_mask];
> + priv->adminq_prod_cnt++;
> + prod_cnt = priv->adminq_prod_cnt;
> +
> + memcpy(cmd, cmd_orig, sizeof(*cmd_orig));
Eh, I guess you don't even need memory barriers around the data movement
to the DMA buffer because of the restriction to x86? :)
> + gve_adminq_kick_cmd(priv, prod_cnt);
> + if (!gve_adminq_wait_for_cmd(priv, prod_cnt)) {
> + dev_err(&priv->pdev->dev, "AQ command timed out, need to reset AQ\n");
> + return -ENOTRECOVERABLE;
> + }
> + memcpy(cmd_orig, cmd, sizeof(*cmd));
> + status = be32_to_cpu(READ_ONCE(cmd->status));
> + return gve_adminq_parse_err(&priv->pdev->dev, status);
> +}
> +int gve_adminq_describe_device(struct gve_priv *priv)
> +{
> + struct gve_device_descriptor *descriptor;
> + union gve_adminq_command cmd;
> + dma_addr_t descriptor_bus;
> + int err = 0;
> + u8 *mac;
> + u16 mtu;
> +
> + memset(&cmd, 0, sizeof(cmd));
> + descriptor = dma_alloc_coherent(&priv->pdev->dev, PAGE_SIZE,
> + &descriptor_bus, GFP_KERNEL);
> + if (!descriptor)
> + return -ENOMEM;
> + cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESCRIBE_DEVICE);
> + cmd.describe_device.device_descriptor_addr =
> + cpu_to_be64(descriptor_bus);
> + cmd.describe_device.device_descriptor_version =
> + cpu_to_be32(GVE_ADMINQ_DEVICE_DESCRIPTOR_VERSION);
> + cmd.describe_device.available_length = cpu_to_be32(PAGE_SIZE);
> +
> + err = gve_adminq_execute_cmd(priv, &cmd);
> + if (err)
> + goto free_device_descriptor;
> +
> + mtu = be16_to_cpu(descriptor->mtu);
> + if (mtu < ETH_MIN_MTU) {
> + netif_err(priv, drv, priv->dev, "MTU %d below minimum MTU\n",
> + mtu);
> + err = -EINVAL;
> + goto free_device_descriptor;
> + }
> + priv->dev->max_mtu = mtu;
> + priv->num_event_counters = be16_to_cpu(descriptor->counters);
> + ether_addr_copy(priv->dev->dev_addr, descriptor->mac);
Since you check MTU you can check the address with
is_valid_ether_addr().
Also it's common practice to copy the provisioned MAC to
netdev->perm_addr as well as dev_addr for VFs.
> + mac = descriptor->mac;
> + netif_info(priv, drv, priv->dev, "MAC addr: %pM\n", mac);
> +
> +free_device_descriptor:
> + dma_free_coherent(&priv->pdev->dev, sizeof(*descriptor), descriptor,
> + descriptor_bus);
> + return err;
> +}
Oh, okay, David already applied..
^ permalink raw reply
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: David Miller @ 2019-07-02 2:39 UTC (permalink / raw)
To: willy; +Cc: xiyou.wangcong, netdev, dcaratti, chrism
In-Reply-To: <20190702023730.GA1729@bombadil.infradead.org>
From: Matthew Wilcox <willy@infradead.org>
Date: Mon, 1 Jul 2019 19:37:30 -0700
> On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Fri, 28 Jun 2019 11:03:40 -0700
>>
>> > idr_get_next_ul() is problematic by design, it can't handle
>> > the following overflow case well on 32-bit CPU:
>> >
>> > u32 id = UINT_MAX;
>> > idr_alloc_u32(&id);
>> > while (idr_get_next_ul(&id) != NULL)
>> > id++;
>> >
>> > when 'id' overflows and becomes 0 after UINT_MAX, the loop
>> > goes infinite.
>> >
>> > Fix this by eliminating external users of idr_get_next_ul()
>> > and migrating them to idr_for_each_entry_continue_ul(). And
>> > add an additional parameter for these iteration macros to detect
>> > overflow properly.
>> >
>> > Please merge this through networking tree, as all the users
>> > are in networking subsystem.
>>
>> Series applied, thanks Cong.
>
> Ugh, I don't even get the weekend to reply?
>
> I think this is just a bad idea. It'd be better to apply the conversion
> patches to use XArray than fix up this crappy interface. I didn't
> reply before because I wanted to check those patches still apply and
> post them as part of the response. Now they're definitely broken and
> need to be redone.
Please work this out with Cong.
I think his approach is safe for net and thus -stable than an xarray
conversion.
^ permalink raw reply
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: Matthew Wilcox @ 2019-07-02 2:37 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, dcaratti, chrism
In-Reply-To: <20190701.191600.1570499492484804858.davem@davemloft.net>
On Mon, Jul 01, 2019 at 07:16:00PM -0700, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Fri, 28 Jun 2019 11:03:40 -0700
>
> > idr_get_next_ul() is problematic by design, it can't handle
> > the following overflow case well on 32-bit CPU:
> >
> > u32 id = UINT_MAX;
> > idr_alloc_u32(&id);
> > while (idr_get_next_ul(&id) != NULL)
> > id++;
> >
> > when 'id' overflows and becomes 0 after UINT_MAX, the loop
> > goes infinite.
> >
> > Fix this by eliminating external users of idr_get_next_ul()
> > and migrating them to idr_for_each_entry_continue_ul(). And
> > add an additional parameter for these iteration macros to detect
> > overflow properly.
> >
> > Please merge this through networking tree, as all the users
> > are in networking subsystem.
>
> Series applied, thanks Cong.
Ugh, I don't even get the weekend to reply?
I think this is just a bad idea. It'd be better to apply the conversion
patches to use XArray than fix up this crappy interface. I didn't
reply before because I wanted to check those patches still apply and
post them as part of the response. Now they're definitely broken and
need to be redone.
^ permalink raw reply
* Re: [PATCH net-next v4 0/4] Add gve driver
From: David Miller @ 2019-07-02 2:36 UTC (permalink / raw)
To: csully; +Cc: netdev, lkp, julia.lawall
In-Reply-To: <20190701225755.209250-1-csully@google.com>
From: Catherine Sullivan <csully@google.com>
Date: Mon, 1 Jul 2019 15:57:51 -0700
> This patch series adds the gve driver which will support the
> Compute Engine Virtual NIC that will be available in the future.
...
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCHv3 next 0/3] blackhole device to invalidate dst
From: David Miller @ 2019-07-02 2:35 UTC (permalink / raw)
To: maheshb; +Cc: netdev, edumazet, michael.chan, dja, mahesh
In-Reply-To: <20190701213843.102002-1-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Mon, 1 Jul 2019 14:38:43 -0700
...
> The idea here is to not alter the data-path with additional
> locks or smb()/rmb() barriers to avoid racy assignments but
> to create a new device that has really low MTU that has
> .ndo_start_xmit essentially a kfree_skb(). Make use of this
> device instead of 'lo' when marking the dst dead.
>
> First patch implements the blackhole device and second
> patch uses it in IPv4 and IPv6 stack while the third patch
> is the self test that ensures the sanity of this device.
...
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] net: ethernet: broadcom: bcm63xx_enet: Remove unneeded memset
From: David Miller @ 2019-07-02 2:29 UTC (permalink / raw)
To: hariprasad.kelam
Cc: f.fainelli, bcm-kernel-feedback-list, andrew, gregkh, yuehaibing,
tglx, mcgrof, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20190630142949.GA7422@hari-Inspiron-1545>
From: Hariprasad Kelam <hariprasad.kelam@gmail.com>
Date: Sun, 30 Jun 2019 19:59:49 +0530
> Remove unneeded memset as alloc_etherdev is using kvzalloc which uses
> __GFP_ZERO flag
>
> Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: santosh.shilimkar @ 2019-07-02 2:28 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <14c34ac2-38ed-9d51-f27d-74120ff34c54@oracle.com>
On 7/1/19 2:06 PM, Gerd Rausch wrote:
> Hi Santosh,
>
> On 01/07/2019 14.00, santosh.shilimkar@oracle.com wrote:
>>>
>> Look for command timeout in CX3 sources. 60 second is upper bound in
>> CX3. Its not standard in specs(at least not that I know) though
>> and may vary from vendor to vendor.
>>
>
> I am not seeing it. Can you point me to the right place?
>
Below. All command timeouts are 60 seconds.
enum {
MLX4_CMD_TIME_CLASS_A = 60000,
MLX4_CMD_TIME_CLASS_B = 60000,
MLX4_CMD_TIME_CLASS_C = 60000,
};
But having said that, I re-looked the code you are patching
and thats actually only FRWR code which is purely work-request
based so this command timeout shouldn't matter.
If the work request fails, then it will lead to flush errors and
MRs will be marked as STALE. So this wait may not be necessary
There is a socket call RDS_GET_MR which needs to be synchronous
and that Avinash has actually fixed by making this MR registration
processes synchronous. Inline registration is still kept async.
RDS_GET_MR case is what actually showing the issue you saw
and the fix for that Avinash has it in production kernel.
I believe with that change, registration issue becomes non-issue
already.
And as far as invalidation concerned with proxy qp, it not longer
races with data path qp.
May be you can try those changes if not already to see if it
addresses the couple of cases where you ended up adding
timeouts.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH 0/3, net-next, v2] net: netsec: Add XDP Support
From: David Miller @ 2019-07-02 2:27 UTC (permalink / raw)
To: ilias.apalodimas
Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, brouer, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, maciejromanfijalkowski
In-Reply-To: <1561785805-21647-1-git-send-email-ilias.apalodimas@linaro.org>
From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date: Sat, 29 Jun 2019 08:23:22 +0300
> This is a respin of https://www.spinics.net/lists/netdev/msg526066.html
> Since page_pool API fixes are merged into net-next we can now safely use
> it's DMA mapping capabilities.
>
> First patch changes the buffer allocation from napi/netdev_alloc_frag()
> to page_pool API. Although this will lead to slightly reduced performance
> (on raw packet drops only) we can use the API for XDP buffer recycling.
> Another side effect is a slight increase in memory usage, due to using a
> single page per packet.
>
> The second patch adds XDP support on the driver.
> There's a bunch of interesting options that come up due to the single
> Tx queue.
> Locking is needed(to avoid messing up the Tx queues since ndo_xdp_xmit
> and the normal stack can co-exist). We also need to track down the
> 'buffer type' for TX and properly free or recycle the packet depending
> on it's nature.
>
>
> Changes since RFC:
> - Bug fixes from Jesper and Maciej
> - Added page pool API to retrieve the DMA direction
>
> Changes since v1:
> - Use page_pool_free correctly if xdp_rxq_info_reg() failed
Series applied, thanks.
I realize from the discussion on patch #3 there will be follow-ups to this.
^ permalink raw reply
* Re: [PATCH net] net/tls: make sure offload also gets the keys wiped
From: David Miller @ 2019-07-02 2:23 UTC (permalink / raw)
To: jakub.kicinski
Cc: john.fastabend, netdev, oss-drivers, alexei.starovoitov, sd,
dirk.vandermerwe
In-Reply-To: <20190628231139.16842-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 28 Jun 2019 16:11:39 -0700
> Commit 86029d10af18 ("tls: zero the crypto information from tls_context
> before freeing") added memzero_explicit() calls to clear the key material
> before freeing struct tls_context, but it missed tls_device.c has its
> own way of freeing this structure. Replace the missing free.
>
> Fixes: 86029d10af18 ("tls: zero the crypto information from tls_context before freeing")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net/tls: reject offload of TLS 1.3
From: David Miller @ 2019-07-02 2:22 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, oss-drivers, borisp, alexei.starovoitov, dirk.vandermerwe
In-Reply-To: <20190628230759.16360-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 28 Jun 2019 16:07:59 -0700
> Neither drivers nor the tls offload code currently supports TLS
> version 1.3. Check the TLS version when installing connection
> state. TLS 1.3 will just fallback to the kernel crypto for now.
>
> Fixes: 130b392c6cd6 ("net: tls: Add tls 1.3 support")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Applied and queued up for v5.1+ -stable.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tc-testing: added tdc tests for prio qdisc
From: David Miller @ 2019-07-02 2:21 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1561757521-15439-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Fri, 28 Jun 2019 17:32:01 -0400
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Fix batched event generation for mirred action
From: David Miller @ 2019-07-02 2:18 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1561746618-29349-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Fri, 28 Jun 2019 14:30:16 -0400
> When adding or deleting a batch of entries, the kernel sends upto
> TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
> consider that the action sizes may vary and require different skb sizes.
>
> For example :
>
> % cat tc-batch.sh
> TC="sudo /mnt/iproute2.git/tc/tc"
>
> $TC actions flush action mirred
> for i in `seq 1 $1`;
> do
> cmd="action mirred egress redirect dev lo index $i "
> args=$args$cmd
> done
> $TC actions add $args
> %
> % ./tc-batch.sh 32
> Error: Failed to fill netlink attributes while adding TC action.
> We have an error talking to the kernel
> %
>
> patch 1 adds callback in tc_action_ops of mirred action, which calculates
> the action size, and passes size to tcf_add_notify()/tcf_del_notify().
>
> patch 2 updates the TDC test suite with relevant test cases.
Series applied, thanks.
^ permalink raw reply
* Re: [Patch net 0/3] idr: fix overflow cases on 32-bit CPU
From: David Miller @ 2019-07-02 2:16 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, dcaratti, chrism, willy
In-Reply-To: <20190628180343.8230-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 28 Jun 2019 11:03:40 -0700
> idr_get_next_ul() is problematic by design, it can't handle
> the following overflow case well on 32-bit CPU:
>
> u32 id = UINT_MAX;
> idr_alloc_u32(&id);
> while (idr_get_next_ul(&id) != NULL)
> id++;
>
> when 'id' overflows and becomes 0 after UINT_MAX, the loop
> goes infinite.
>
> Fix this by eliminating external users of idr_get_next_ul()
> and migrating them to idr_for_each_entry_continue_ul(). And
> add an additional parameter for these iteration macros to detect
> overflow properly.
>
> Please merge this through networking tree, as all the users
> are in networking subsystem.
Series applied, thanks Cong.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2019-07-02 2:13 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List, Matteo Croce,
Florian Westphal
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/ipv4/devinet.c
between commit:
2e6054636816 ("ipv4: don't set IPv6 only flags to IPv4 addresses")
from the net tree and commit:
2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/ipv4/devinet.c
index c5ebfa199794,137d1892395d..000000000000
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@@ -473,11 -482,10 +487,13 @@@ static int __inet_insert_ifa(struct in_
ifa->ifa_flags &= ~IFA_F_SECONDARY;
last_primary = &in_dev->ifa_list;
+ /* Don't set IPv6 only flags to IPv4 addresses */
+ ifa->ifa_flags &= ~IPV6ONLY_FLAGS;
+
- for (ifap = &in_dev->ifa_list; (ifa1 = *ifap) != NULL;
- ifap = &ifa1->ifa_next) {
+ ifap = &in_dev->ifa_list;
+ ifa1 = rtnl_dereference(*ifap);
+
+ while (ifa1) {
if (!(ifa1->ifa_flags & IFA_F_SECONDARY) &&
ifa->ifa_scope <= ifa1->ifa_scope)
last_primary = &ifa1->ifa_next;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [net-next 1/1] tipc: embed jiffies in macro TIPC_BC_RETR_LIM
From: David Miller @ 2019-07-02 2:12 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, gordan.mihaljevic, tung.q.nguyen, hoang.h.le, canh.d.luu,
ying.xue, tipc-discussion
In-Reply-To: <1561734380-26868-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Fri, 28 Jun 2019 17:06:20 +0200
> The macro TIPC_BC_RETR_LIM is always used in combination with 'jiffies',
> so we can just as well perform the addition in the macro itself. This
> way, we get a few shorter code lines and one less line break.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] hinic: implement the statistical interface of ethtool
From: Jakub Kicinski @ 2019-07-02 2:12 UTC (permalink / raw)
To: Xue Chaojing
Cc: davem, linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun,
chiqijun, wulike1, Stephen Hemminger
In-Reply-To: <20190624120519.4ec22e19@cakuba.netronome.com>
On Mon, 24 Jun 2019 12:05:19 -0700, Jakub Kicinski wrote:
> On Mon, 24 Jun 2019 03:50:12 +0000, Xue Chaojing wrote:
> > diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> > index be28a9a7f033..8d98f37c88a8 100644
> > --- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> > +++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> > @@ -438,6 +438,344 @@ static u32 hinic_get_rxfh_indir_size(struct net_device *netdev)
> > return HINIC_RSS_INDIR_SIZE;
> > }
> >
> > +#define ARRAY_LEN(arr) ((int)((int)sizeof(arr) / (int)sizeof(arr[0])))
> > +
> > +#define HINIC_NETDEV_STAT(_stat_item) { \
> > + .name = #_stat_item, \
> > + .size = FIELD_SIZEOF(struct rtnl_link_stats64, _stat_item), \
> > + .offset = offsetof(struct rtnl_link_stats64, _stat_item) \
> > +}
> > +
> > +static struct hinic_stats hinic_netdev_stats[] = {
> > + HINIC_NETDEV_STAT(rx_packets),
> > + HINIC_NETDEV_STAT(tx_packets),
> > + HINIC_NETDEV_STAT(rx_bytes),
> > + HINIC_NETDEV_STAT(tx_bytes),
> > + HINIC_NETDEV_STAT(rx_errors),
> > + HINIC_NETDEV_STAT(tx_errors),
> > + HINIC_NETDEV_STAT(rx_dropped),
> > + HINIC_NETDEV_STAT(tx_dropped),
> > + HINIC_NETDEV_STAT(multicast),
> > + HINIC_NETDEV_STAT(collisions),
> > + HINIC_NETDEV_STAT(rx_length_errors),
> > + HINIC_NETDEV_STAT(rx_over_errors),
> > + HINIC_NETDEV_STAT(rx_crc_errors),
> > + HINIC_NETDEV_STAT(rx_frame_errors),
> > + HINIC_NETDEV_STAT(rx_fifo_errors),
> > + HINIC_NETDEV_STAT(rx_missed_errors),
> > + HINIC_NETDEV_STAT(tx_aborted_errors),
> > + HINIC_NETDEV_STAT(tx_carrier_errors),
> > + HINIC_NETDEV_STAT(tx_fifo_errors),
> > + HINIC_NETDEV_STAT(tx_heartbeat_errors),
> > +};
>
> I think we wanted to stop duplicating standard netdev stats in ethtool
> -S. Chaojing please post a patch to remove this part, the other stats
> are good.
Please address this comment or I will send a revert of the entire patch.
^ permalink raw reply
* Re: [PATCH] netlink: use 48 byte ctx instead of 6 signed longs for callback
From: David Miller @ 2019-07-02 2:12 UTC (permalink / raw)
To: johannes; +Cc: Jason, netdev, linux-kernel
In-Reply-To: <0092b0b405e02ac7401ceaad2ea650abc44559ea.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 28 Jun 2019 16:42:26 +0200
> On Fri, 2019-06-28 at 16:40 +0200, Jason A. Donenfeld wrote:
>> People are inclined to stuff random things into cb->args[n] because it
>> looks like an array of integers. Sometimes people even put u64s in there
>> with comments noting that a certain member takes up two slots. The
>> horror! Really this should mirror the usage of skb->cb, which are just
>> 48 opaque bytes suitable for casting a struct. Then people can create
>> their usual casting macros for accessing strongly typed members of a
>> struct.
>>
>> As a plus, this also gives us the same amount of space on 32bit and 64bit.
>>
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> Cc: Johannes Berg <johannes@sipsolutions.net>
>
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net v3] vxlan: do not destroy fdb if register_netdevice() is failed
From: David Miller @ 2019-07-02 2:06 UTC (permalink / raw)
To: ap420073; +Cc: roopa, petrm, netdev
In-Reply-To: <20190628050725.9445-1-ap420073@gmail.com>
From: Taehee Yoo <ap420073@gmail.com>
Date: Fri, 28 Jun 2019 14:07:25 +0900
> __vxlan_dev_create() destroys FDB using specific pointer which indicates
> a fdb when error occurs.
> But that pointer should not be used when register_netdevice() fails because
> register_netdevice() internally destroys fdb when error occurs.
>
> This patch makes vxlan_fdb_create() to do not link fdb entry to vxlan dev
> internally.
> Instead, a new function vxlan_fdb_insert() is added to link fdb to vxlan
> dev.
>
> vxlan_fdb_insert() is called after calling register_netdevice().
> This routine can avoid situation that ->ndo_uninit() destroys fdb entry
> in error path of register_netdevice().
> Hence, error path of __vxlan_dev_create() routine can have an opportunity
> to destroy default fdb entry by hand.
>
> Test command
> ip link add bonding_masters type vxlan id 0 group 239.1.1.1 \
> dev enp0s9 dstport 4789
>
> Splat looks like:
...
> Fixes: 0241b836732f ("vxlan: fix default fdb entry netlink notify ordering during netdev create")
> Suggested-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH net-next] net/ipv6: Fix misuse of proc_dointvec "flowlabel_reflect"
From: David Miller @ 2019-07-02 2:05 UTC (permalink / raw)
To: devel; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20190628023714.1923-1-devel@etsukata.com>
From: Eiichi Tsukata <devel@etsukata.com>
Date: Fri, 28 Jun 2019 11:37:14 +0900
> /proc/sys/net/ipv6/flowlabel_reflect assumes written value to be in the
> range of 0 to 3. Use proc_dointvec_minmax instead of proc_dointvec.
>
> Fixes: 323a53c41292 ("ipv6: tcp: enable flowlabel reflection in some RST packets")
> Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: link_watch: prevent starvation when processing linkwatch wq
From: David Miller @ 2019-07-02 2:03 UTC (permalink / raw)
To: linyunsheng
Cc: hkallweit1, gregkh, tglx, netdev, linux-kernel, pbonzini, rkrcmar,
kvm
In-Reply-To: <1561684399-235123-1-git-send-email-linyunsheng@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Fri, 28 Jun 2019 09:13:19 +0800
> When user has configured a large number of virtual netdev, such
> as 4K vlans, the carrier on/off operation of the real netdev
> will also cause it's virtual netdev's link state to be processed
> in linkwatch. Currently, the processing is done in a work queue,
> which may cause rtnl locking starvation problem and worker
> starvation problem for other work queue, such as irqfd_inject wq.
>
> This patch releases the cpu when link watch worker has processed
> a fixed number of netdev' link watch event, and schedule the
> work queue again when there is still link watch event remaining.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> ---
> V2: use cond_resched and rtnl_unlock after processing a fixed
> number of events
> V3: fall back to v1 and change commit log to reflect that.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
From: David Miller @ 2019-07-02 2:02 UTC (permalink / raw)
To: marcelo.leitner; +Cc: netdev, lucien.xin, nhorman, linux-sctp, hdanton
In-Reply-To: <bcbc85604e53843a731a79df620d5f92b194d085.1561675505.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 27 Jun 2019 19:48:10 -0300
> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated. When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail. In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state. On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
>
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
>
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
>
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [Patch net] netrom: fix a memory leak in nr_rx_frame()
From: David Miller @ 2019-07-02 2:01 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, syzbot+d6636a36d3c34bd88938
In-Reply-To: <20190627213058.3028-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 27 Jun 2019 14:30:58 -0700
> When the skb is associated with a new sock, just assigning
> it to skb->sk is not sufficient, we have to set its destructor
> to free the sock properly too.
>
> Reported-by: syzbot+d6636a36d3c34bd88938@syzkaller.appspotmail.com
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] mlxsw: PTP timestamping support
From: David Miller @ 2019-07-02 1:59 UTC (permalink / raw)
To: idosch; +Cc: netdev, richardcochran, jiri, petrm, mlxsw, idosch
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Sun, 30 Jun 2019 09:04:44 +0300
> This is the second patchset adding PTP support in mlxsw. Next patchset
> will add PTP shapers which are required to maintain accuracy under rates
> lower than 40Gb/s, while subsequent patchsets will add tracepoints and
> selftests.
...
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-07-02 1:59 UTC (permalink / raw)
To: Song Liu
Cc: Andy Lutomirski, linux-security@vger.kernel.org, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API, Kees Cook
In-Reply-To: <0DE7F23E-9CD2-4F03-82B5-835506B59056@fb.com>
On Mon, Jul 1, 2019 at 2:03 AM Song Liu <songliubraving@fb.com> wrote:
>
> Hi Andy,
>
> Thanks for these detailed analysis.
>
> > On Jun 30, 2019, at 8:12 AM, Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Fri, Jun 28, 2019 at 12:05 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> Hi Andy,
> >>
> >>> On Jun 27, 2019, at 4:40 PM, Andy Lutomirski <luto@kernel.org> wrote:
> >>>
> >>> On 6/27/19 1:19 PM, Song Liu wrote:
> >>>> This patch introduce unprivileged BPF access. The access control is
> >>>> achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> >>>> to call sys_bpf().
> >>>> Two ioctl command are added to /dev/bpf:
> >>>> The two commands enable/disable permission to call sys_bpf() for current
> >>>> task. This permission is noted by bpf_permitted in task_struct. This
> >>>> permission is inherited during clone(CLONE_THREAD).
> >>>> Helper function bpf_capable() is added to check whether the task has got
> >>>> permission via /dev/bpf.
> >>>
> >>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >>>> index 0e079b2298f8..79dc4d641cf3 100644
> >>>> --- a/kernel/bpf/verifier.c
> >>>> +++ b/kernel/bpf/verifier.c
> >>>> @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
> >>>> env->insn_aux_data[i].orig_idx = i;
> >>>> env->prog = *prog;
> >>>> env->ops = bpf_verifier_ops[env->prog->type];
> >>>> - is_priv = capable(CAP_SYS_ADMIN);
> >>>> + is_priv = bpf_capable(CAP_SYS_ADMIN);
> >>>
> >>> Huh? This isn't a hardening measure -- the "is_priv" verifier mode allows straight-up leaks of private kernel state to user mode.
> >>>
> >>> (For that matter, the pending lockdown stuff should possibly consider this a "confidentiality" issue.)
> >>>
> >>>
> >>> I have a bigger issue with this patch, though: it's a really awkward way to pretend to have capabilities. For bpf, it seems like you could make this be a *real* capability without too much pain since there's only one syscall there. Just find a way to pass an fd to /dev/bpf into the syscall. If this means you need a new bpf_with_cap() syscall that takes an extra argument, so be it. The old bpf() syscall can just translate to bpf_with_cap(..., -1).
> >>>
> >>> For a while, I've considered a scheme I call "implicit rights". There would be a directory in /dev called /dev/implicit_rights. This would either be part of devtmpfs or a whole new filesystem -- it would *not* be any other filesystem. The contents would be files that can't be read or written and exist only in memory. You create them with a privileged syscall. Certain actions that are sensitive but not at the level of CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user namespaces, profiling the kernel, etc) could require an "implicit right". When you do them, if you don't have CAP_SYS_ADMIN, the kernel would do a path walk for, say, /dev/implicit_rights/bpf and, if the object exists, can be opened, and actually refers to the "bpf" rights object, then the action is allowed. Otherwise it's denied.
> >>>
> >>> This is extensible, and it doesn't require the rather ugly per-task state of whether it's enabled.
> >>>
> >>> For things like creation of user namespaces, there's an existing API, and the default is that it works without privilege. Switching it to an implicit right has the benefit of not requiring code changes to programs that already work as non-root.
> >>>
> >>> But, for BPF in particular, this type of compatibility issue doesn't exist now. You already can't use most eBPF functionality without privilege. New bpf-using programs meant to run without privilege are *new*, so they can use a new improved API. So, rather than adding this obnoxious ioctl, just make the API explicit, please.
> >>>
> >>> Also, please cc: linux-abi next time.
> >>
> >> Thanks for your inputs.
> >>
> >> I think we need to clarify the use case here. In this case, we are NOT
> >> thinking about creating new tools for unprivileged users. Instead, we
> >> would like to use existing tools without root.
> >
> > I read patch 4, and I interpret it very differently. Patches 2-4 are
> > creating a new version of libbpf and a new version of bpftool. Given
> > this, I see no real justification for adding a new in-kernel per-task
> > state instead of just pushing the complexity into libbpf.
>
> I am not sure whether we are on the same page. Let me try an example,
> say we have application A, which calls sys_bpf().
>
> Before the series: we have to run A with root;
> After the series: we add a special user with access to /dev/bpf, and
> run A with this special user.
>
> If we look at the whole system, I would say we are more secure after
> the series.
>
> I am not trying to make an extreme example here, because this use case
> is the motivation here.
>
> To stay safe, we have to properly manage the permission of /dev/bpf.
> This is just like we need to properly manage access to /etc/sudoers and
> /dev/mem.
>
> Does this make sense?
>
I think I'm understanding your motivation. You're not trying to make
bpf() generically usable without privilege -- you're trying to create
a way to allow certain users to access dangerous bpf functionality
within some limits.
That's a perfectly fine goal, but I think you're reinventing the
wheel, and the wheel you're reinventing is quite complicated and
already exists. I think you should teach bpftool to be secure when
installed setuid root or with fscaps enabled and put your policy in
bpftool. If you want to harden this a little bit, it would seem
entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
capabilities that they currently check.
Your example of /etc/sudoers is apt, and it does not involve any
kernel support :)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).