* Re: [PATCH net-next 1/2] mqprio: Add a new hardware offload type in mqprio
From: Yunsheng Lin @ 2017-10-13 2:12 UTC (permalink / raw)
To: Yuval Mintz, davem@davemloft.net
Cc: huangdaode@hisilicon.com, xuwei5@hisilicon.com,
liguozhu@hisilicon.com, Yisen.Zhuang@huawei.com,
gabriele.paoloni@huawei.com, john.garry@huawei.com,
linuxarm@huawei.com, salil.mehta@huawei.com, lipeng321@huawei.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <AM0PR0502MB3683208C0F411F67C70BF6B1BF4B0@AM0PR0502MB3683.eurprd05.prod.outlook.com>
Hi, Yuval
On 2017/10/13 4:10, Yuval Mintz wrote:
>> When a driver supports both dcb and hardware offloaded mqprio, and
>> user is running mqprio and dcb tool concurrently, the configuration
>> set by each tool may be conflicted with each other because the dcb
> (for second 'each') s/each/the
>
Will do, Thanks
>> and mqprio may be using the same hardwere offload component and share
> s/hardwere/hardware
Will do, Thanks
>
>> the tc system in the network stack.
>>
>> This patch adds a new offload type to indicate that the underlying
>> driver offload prio mapping as part of DCB. If the driver would be
> 'should' offload
Will do, Thanks
>
>> incapable of that it would refuse the offload. User would then have
>> to explicitly request that qdisc offload.
>
>
>
^ permalink raw reply
* Re: [PATCH net-next v2 1/1] bridge: return error code when deleting Vlan
From: Nikolay Aleksandrov @ 2017-10-13 2:15 UTC (permalink / raw)
To: Jamal Hadi Salim, Roman Mashak, David Ahern
Cc: David Miller, Stephen Hemminger, Linux Kernel Network Developers
In-Reply-To: <64292550-f043-c1f7-5b0e-004288832887@mojatatu.com>
On 13.10.2017 05:03, Jamal Hadi Salim wrote:
> On 17-10-12 02:12 PM, Nikolay Aleksandrov wrote:
>> On 12/10/17 21:07, Roman Mashak wrote:
>
>>> For example, if you attempt to delete a non-existing vlan on a port,
>>> the current code succeeds and also sends event :
>>>
>>> rtnetlink_rcv_msg
>>> rtnl_bridge_dellink
>>> br_dellink
>>> br_afspec
>>> br_vlan_info
>>>
>>> int br_dellink(..)
>>> {
>>> ...
>>> err = br_afspec()
>>> if (err == 0)
>>> br_ifinfo_notify(RTM_NEWLINK, p);
>>> }
>>>
>>> This is misleading, so a proper errcode has to be produced.
>>>
>>
>
>
>
>> True, but you also change the expected behaviour because now a user can
>> clear all vlans with one request (1 - 4094), and after the change that
>> will fail with a partial delete if some vlan was missing.
>>
>
> The issue is more subtle (per Roman above):
> Try to delete a vlan (that doesnt exist).
> 1) It says "success".
> 2) Worse: Another process listening (bridge monitor?) gets an _event_
> that the vlan has been deleted (when it never existed in the first
> place).
>
>> This has been the behaviour forever and some script might depend on it.
>> Also IMO, and as David also mentioned, doing a partial delete is not
>> good.
>>
>
> I think this is a bug (especially the event part).
>
> cheers,
> jamal
>
Fair enough, but after the patch you get the opposite effect too - you
delete a couple of vlans but you don't generate an event because of an
error in the middle. That at least can be taken care of.
I do agree it's a bug, but there might be scripts that rely on it and
don't check the return value when clearing vlans. They will end up with
a partial clear and wrongly assumed state, so maybe leave the
opportunistic delete but count if anything was actually deleted and send
an event only then ?
That should make everyone happy :-)
^ permalink raw reply
* linux-next: build failure after merge of the akpm-current tree
From: Mark Brown @ 2017-10-13 2:38 UTC (permalink / raw)
To: Andrew Morton, Jakub Kicinski, Simon Horman, David S. Miller
Cc: netdev, Linux-Next Mailing List, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 808 bytes --]
Hi Andrew,
After merging the akpm-current tree, today's linux-next build
(x86 allmodconfig) failed like this:
CC [M] drivers/net/ethernet/netronome/nfp/nfp_app.o
In file included from /home/broonie/tmpfs/next/drivers/net/ethernet/netronome/nfp/nfp_asm.c:40:0:
/home/broonie/tmpfs/next/drivers/net/ethernet/netronome/nfp/nfp_asm.h: In function '__enc_swreg_lm':
/home/broonie/tmpfs/next/drivers/net/ethernet/netronome/nfp/nfp_asm.h:301:2: error: implicit declaration of function 'WARN_ON' [-Werror=implicit-function-declaration]
WARN_ON(id > 3 || (off && mode != NN_LM_MOD_NONE));
^
cc1: some warnings being treated as errors
Caused by some reliance on an implicit include being exposed by a header
reorganization in your tree. I'll add a patch for this which I'll post,
probably tomorrow morning.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [next-queue PATCH v7 4/6] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: Eric Dumazet @ 2017-10-13 2:42 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: netdev, intel-wired-lan, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
richardcochran, henrik, levipearson, rodney.cummings
In-Reply-To: <20171013004005.17416-5-vinicius.gomes@intel.com>
On Thu, 2017-10-12 at 17:40 -0700, Vinicius Costa Gomes wrote:
> This queueing discipline implements the shaper algorithm defined by
> the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
>
> It's primary usage is to apply some bandwidth reservation to user
> defined traffic classes, which are mapped to different queues via the
> mqprio qdisc.
>
> Only a simple software implementation is added for now.
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> ---
> +/* timediff is in ns, slope is in kbps */
> +static s64 timediff_to_credits(s64 timediff, s32 slope)
> +{
> + s64 credits = timediff * slope * BYTES_PER_KBIT;
> +
> + do_div(credits, NSEC_PER_SEC);
> +
> + return credits;
> +}
> +
> +static s64 delay_from_credits(s64 credits, s32 slope)
> +{
> + s64 rate = slope * BYTES_PER_KBIT;
> + s64 delay;
> +
> + if (unlikely(rate == 0))
> + return S64_MAX;
> +
> + delay = -credits * NSEC_PER_SEC;
> + do_div(delay, rate);
> +
> + return delay;
> +}
> +
> +static s64 credits_from_len(unsigned int len, s32 slope, s64 port_rate)
> +{
> + /* As do_div() only works on unsigned quantities, convert
> + * slope to a positive number here, and credits to a negative
> + * number before returning.
> + */
> + s64 rate = -slope * BYTES_PER_KBIT;
> + s64 credits;
> +
> + if (unlikely(port_rate == 0))
> + return S64_MAX;
> +
> + credits = len * rate;
> + do_div(credits, port_rate);
> +
> + return -credits;
> +}
> +
Your mixing of s64 and u64 is disturbing.
do_div() handles u64, not s64.
div64_s64() might be needed in place of do_div()
^ permalink raw reply
* [PATCH] nfp: Explicitly include linux/bug.h
From: Mark Brown @ 2017-10-13 2:50 UTC (permalink / raw)
To: davem, Jakub Kicinski, Simon Horman; +Cc: oss-drivers, netdev, Mark Brown
Today's -next build encountered an error due to a missing definition of
WARN_ON(), caused by some header reorganization removing an implicit
inclusion of linux/bug.h. Fix this with an explicit inclusion.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/net/ethernet/netronome/nfp/nfp_app.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.c b/drivers/net/ethernet/netronome/nfp/nfp_app.c
index 82c290763529..5d9e2eba5b49 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.c
@@ -31,6 +31,7 @@
* SOFTWARE.
*/
+#include <linux/bug.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_asm.h b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
index c4c18dd5630a..aa397bf308e4 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_asm.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_asm.h
@@ -35,6 +35,7 @@
#define __NFP_ASM_H__ 1
#include <linux/bitfield.h>
+#include <linux/bug.h>
#include <linux/types.h>
#define REG_NONE 0
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] nfp: Explicitly include linux/bug.h
From: Jakub Kicinski @ 2017-10-13 3:16 UTC (permalink / raw)
To: Mark Brown; +Cc: davem, Simon Horman, oss-drivers, netdev
In-Reply-To: <20171013025035.4450-1-broonie@kernel.org>
On Fri, 13 Oct 2017 03:50:35 +0100, Mark Brown wrote:
> Today's -next build encountered an error due to a missing definition of
> WARN_ON(), caused by some header reorganization removing an implicit
> inclusion of linux/bug.h. Fix this with an explicit inclusion.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thank you!
^ permalink raw reply
* [PATCH v3] net: ftgmac100: Request clock and set speed
From: Joel Stanley @ 2017-10-13 4:16 UTC (permalink / raw)
To: David S . Miller, Benjamin Herrenschmidt
Cc: netdev, linux-kernel, Andrew Jeffery
According to the ASPEED datasheet, gigabit speeds require a clock of
100MHz or higher. Other speeds require 25MHz or higher. This patch
configures a 100MHz clock if the system has a direct-attached
PHY, or 25MHz if the system is running NC-SI which is limited to 100MHz.
There appear to be no other upstream users of the FTGMAC100 driver it is
hard to know the clocking requirements of other platforms. Therefore a
conservative approach was taken with enabling clocks. If the platform is
not ASPEED, both requesting the clock and configuring the speed is
skipped.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
Andrew, can you please give this one a spin on hardware?
v3:
- Fix errors from v2
v2:
- only touch the clocks on Aspeed platforms
- unconditionally call clk_unprepare_disable
drivers/net/ethernet/faraday/ftgmac100.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 9ed8e4b81530..78db8e62a83f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -21,6 +21,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/clk.h>
#include <linux/dma-mapping.h>
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
@@ -59,6 +60,9 @@
/* Min number of tx ring entries before stopping queue */
#define TX_THRESHOLD (MAX_SKB_FRAGS + 1)
+#define FTGMAC_100MHZ 100000000
+#define FTGMAC_25MHZ 25000000
+
struct ftgmac100 {
/* Registers */
struct resource *res;
@@ -96,6 +100,7 @@ struct ftgmac100 {
struct napi_struct napi;
struct work_struct reset_task;
struct mii_bus *mii_bus;
+ struct clk *clk;
/* Link management */
int cur_speed;
@@ -1734,6 +1739,22 @@ static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
nd->link_up ? "up" : "down");
}
+static void ftgmac100_setup_clk(struct ftgmac100 *priv)
+{
+ priv->clk = devm_clk_get(priv->dev, NULL);
+ if (IS_ERR(priv->clk))
+ return;
+
+ clk_prepare_enable(priv->clk);
+
+ /* Aspeed specifies a 100MHz clock is required for up to
+ * 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
+ * is sufficient
+ */
+ clk_set_rate(priv->clk, priv->use_ncsi ? FTGMAC_25MHZ :
+ FTGMAC_100MHZ);
+}
+
static int ftgmac100_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -1830,6 +1851,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
goto err_setup_mdio;
}
+ if (priv->is_aspeed)
+ ftgmac100_setup_clk(priv);
+
/* Default ring sizes */
priv->rx_q_entries = priv->new_rx_q_entries = DEF_RX_QUEUE_ENTRIES;
priv->tx_q_entries = priv->new_tx_q_entries = DEF_TX_QUEUE_ENTRIES;
@@ -1883,6 +1907,8 @@ static int ftgmac100_remove(struct platform_device *pdev)
unregister_netdev(netdev);
+ clk_disable_unprepare(priv->clk);
+
/* There's a small chance the reset task will have been re-queued,
* during stop, make sure it's gone before we free the structure.
*/
--
2.14.1
^ permalink raw reply related
* Waiting for your response to my numerous un-replied emails to you concerning your family inheritance fund ($7.5 million dollars). I seek your assistance and I assured of your capability to champion this business opportunity to remit $7.5 million dollars into your account, if you are interested let me know so that I can send you the details of this transaction. I agreed that 55% of this money will be for you 5% will be set aside for expenses incurred during the business and 40% would be for me. And if you are not willing to handle the transaction don’t border to reply me.
From: Enu Ofe @ 2017-10-13 4:23 UTC (permalink / raw)
^ permalink raw reply
* Waiting for your response to my numerous un-replied emails to you concerning your family inheritance fund ($7.5 million dollars). I seek your assistance and I assured of your capability to champion this business opportunity to remit $7.5 million dollars into your account, if you are interested let me know so that I can send you the details of this transaction. I agreed that 55% of this money will be for you 5% will be set aside for expenses incurred during the business and 40% would be for me. And if you are not willing to handle the transaction don’t border to reply me.
From: Enu Ofe @ 2017-10-13 4:24 UTC (permalink / raw)
^ permalink raw reply
* Re: [PATCH net-next 0/2] Fix IFE meta modules loading
From: David Miller @ 2017-10-13 5:13 UTC (permalink / raw)
To: mrv; +Cc: netdev, jhs
In-Reply-To: <85lgkgw19o.fsf@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Thu, 12 Oct 2017 16:37:39 -0400
> David Miller <davem@davemloft.net> writes:
>
>> From: Roman Mashak <mrv@mojatatu.com>
>> Date: Wed, 11 Oct 2017 10:50:28 -0400
>>
>>> Adjust module alias names of IFE meta modules and fix the bug that
>>> prevented auto-loading IFE modules in run-time.
>>
>> Anyone using the existing alises will be broken by these changes
>> no?
>
> Actually aliases never worked, the bug existed since the day act_meta_*
> modules have been introduced. I suspect everyone compiles them in-kernel
> rather then as modules.
Fair enough, series applied, thanks for explaining.
^ permalink raw reply
* Re: [PATCH net-next 0/3] sched: act: ife: UAPI checks and performance tweaks
From: David Miller @ 2017-10-13 5:23 UTC (permalink / raw)
To: aring; +Cc: jhs, xiyou.wangcong, jiri, netdev, eric.dumazet, bjb
In-Reply-To: <20171011211608.22692-1-aring@mojatatu.com>
From: Alexander Aring <aring@mojatatu.com>
Date: Wed, 11 Oct 2017 17:16:05 -0400
> this patch series contains at first a patch which adds a check for
> IFE_ENCODE and IFE_DECODE when a ife act gets created or updated and adding
> handling of these cases only inside the act callback only.
>
> The second patch use per-cpu counters and move the spinlock around so that
> the spinlock is less being held in act callback.
>
> The last patch use rcu for update parameters and also move the spinlock for
> the same purpose as in patch 2.
>
> Notes:
> - There is still a spinlock around for protecting the metalist and a
> rw-lock for another list. Should be migrated to a rcu list, ife
> possible.
>
> - I use still dereference in dump callback, so I think what I didn't
> got was what happened when rcu_assign_pointer will do when rcu read
> lock is held. I suppose the pointer will be updated, then we don't
> have any issue here.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 0/4] tc-testing: Test suite updates
From: David Miller @ 2017-10-13 5:31 UTC (permalink / raw)
To: lucasb; +Cc: netdev, xiyou.wangcong, jiri, jhs, aring, mrv
In-Reply-To: <1507756614-30333-1-git-send-email-lucasb@mojatatu.com>
From: Lucas Bates <lucasb@mojatatu.com>
Date: Wed, 11 Oct 2017 17:16:50 -0400
> This patch series is a roundup of changes to the tc-testing
> suite:
>
> - Add test cases for police and mirred modules and some coverage
> in already-submitted test categories
> - Break the test case files down into more user-friendly sizes
> - Bug fix to the tdc.py script's handling of the -l argument
Some of the newly added files lack final newlines, and as you can
see in the git patch output a warning is generated.
Please fix this up and resubmit.
Thanks.
^ permalink raw reply
* Re: [PATCH next] ipvlan: always use the current L2 addr of the master
From: David Miller @ 2017-10-13 5:54 UTC (permalink / raw)
To: mahesh; +Cc: netdev, edumazet, maheshb
In-Reply-To: <20171012001626.3255-1-mahesh@bandewar.net>
From: Mahesh Bandewar <mahesh@bandewar.net>
Date: Wed, 11 Oct 2017 17:16:26 -0700
> From: Mahesh Bandewar <maheshb@google.com>
>
> If the underlying master ever changes its L2 (e.g. bonding device),
> then make sure that the IPvlan slaves always emit packets with the
> current L2 of the master instead of the stale mac addr which was
> copied during the device creation. The problem can be seen with
> following script -
>
> #!/bin/bash
> # Create a vEth pair
> ip link add dev veth0 type veth peer name veth1
> ip link set veth0 up
> ip link set veth1 up
> ip link show veth0
> ip link show veth1
> # Create an IPvlan device on one end of this vEth pair.
> ip link add link veth0 dev ipvl0 type ipvlan mode l2
> ip link show ipvl0
> # Change the mac-address of the vEth master.
> ip link set veth0 address 02:11:22:33:44:55
>
> Fixes: 2ad7bf363841 ("ipvlan: Initial check-in of the IPVLAN driver.")
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ip: update policy routing config help
From: David Miller @ 2017-10-13 5:57 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20171012031031.8415-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 11 Oct 2017 20:10:31 -0700
> The kernel config help for policy routing was still pointing at
> an ancient document from 2000 that refers to Linux 2.1. Update it
> to point to something that is at least occasionally updated.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/2] net: support bgmac with B50212E B1 PHY
From: David Miller @ 2017-10-13 5:59 UTC (permalink / raw)
To: zajec5
Cc: netdev, andrew, f.fainelli, hauke, jon.mason, steven.lin1,
tklauser, bcm-kernel-feedback-list, rafal
In-Reply-To: <20171012082126.30660-1-zajec5@gmail.com>
From: Rafał Miłecki <zajec5@gmail.com>
Date: Thu, 12 Oct 2017 10:21:24 +0200
> From: Rafał Miłecki <rafal@milecki.pl>
>
> I got a report that a board with BCM47189 SoC and B50212E B1 PHY doesn't
> work well some devices as there is massive ping loss. After analyzing
> PHY state it has appeared that is runs in slave mode and doesn't auto
> switch to master properly when needed.
>
> This patchset fixes this by:
> 1) Adding new flag support to the PHY driver for setting master mode
> 2) Modifying bgmac to request master mode for reported hardware
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH] ravb: Consolidate clock handling
From: David Miller @ 2017-10-13 6:01 UTC (permalink / raw)
To: geert+renesas
Cc: sergei.shtylyov, horms+renesas, niklas.soderlund+renesas, netdev,
linux-renesas-soc
In-Reply-To: <1507796693-14268-1-git-send-email-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Thu, 12 Oct 2017 10:24:53 +0200
> The module clock is used for two purposes:
> - Wake-on-LAN (WoL), which is optional,
> - gPTP Timer Increment (GTI) configuration, which is mandatory.
>
> As the clock is needed for GTI configuration anyway, WoL is always
> available. Hence remove duplication and repeated obtaining of the clock
> by making GTI use the stored clock for WoL use.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/8] cxgb4: add support to get hardware debug logs via ethtool
From: David Miller @ 2017-10-13 6:04 UTC (permalink / raw)
To: rahul.lakkireddy; +Cc: netdev, ganeshgr, nirranjan, indranil
In-Reply-To: <cover.1507795407.git.rahul.lakkireddy@chelsio.com>
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Date: Thu, 12 Oct 2017 13:54:37 +0530
> This series of patches add support to collect hardware debug logs
> via ethtool --get-dump facility.
There is a lot of global namespace pollution added by these
changes.
A lot of the global symbols you add in this new code have very
poorly namespaced names like "collect_mem_info()"
If the driver is built statically into the kernel this will pollute
the global namespace and conflict with any symbols elsewhere in the
kernel that have the same name.
So please use a proper "cxgb4_" or similar prefix for any non-static
symbols in the driver.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] selftests: rtnetlink: add a small macsec test case
From: David Miller @ 2017-10-13 6:06 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20171012091122.28133-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Thu, 12 Oct 2017 11:11:22 +0200
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/1] net/smc: add SMC rendezvous protocol
From: David Miller @ 2017-10-13 6:06 UTC (permalink / raw)
To: fw
Cc: ubraun, netdev, linux-s390, jwi, schwidefsky, heiko.carstens,
raspl, hwippel
In-Reply-To: <20171012111429.GG26835@breakpoint.cc>
From: Florian Westphal <fw@strlen.de>
Date: Thu, 12 Oct 2017 13:14:29 +0200
> Ursula Braun <ubraun@linux.vnet.ibm.com> wrote:
>> On 10/11/2017 11:06 PM, David Miller wrote:
>> > From: Ursula Braun <ubraun@linux.vnet.ibm.com>
>> > Date: Tue, 10 Oct 2017 16:14:19 +0200
>> >
>> >> The goal of this patch is to leave common TCP code unmodified. Thus,
>> >> it uses netfilter hooks to intercept TCP SYN and SYN/ACK
>> >> packets. For outgoing packets originating from SMC sockets, the
>> >> experimental option is added. For inbound packets destined for SMC
>> >> sockets, the experimental option is checked.
>> >
>> > I think this really isn't going to pass.
>> >
>> > It's a user experience nightmare when the kernel inserts and
>> > deletes filtering rules outside of what the user configures
>> > on their system.
>
> It depends if the hook is passive or not (i.e. mangles
> payload/metadata or returns verdict other than NF_ACCEPT).
>
> OUTPUT hook added here is not passive as it mangles tcp options.
>
>> > This approach was also considerd for ipv6 ILA, and the same
>> > pushback was given.
>
> ahem.
> net/ipv6/ila/ila_xlat.c: err = nf_register_net_hooks(net, ila_nf_hook_ops,
My bad, I thought we had decided against that.
Oh well.
^ permalink raw reply
* Re: [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2017-10-13 6:21 UTC (permalink / raw)
To: David Miller
Cc: netdev, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, dsahern,
alexander.h.duyck, john.fastabend, willemb
In-Reply-To: <20171012.102148.580269505585324793.davem@davemloft.net>
Thu, Oct 12, 2017 at 07:21:48PM CEST, davem@davemloft.net wrote:
>
>Jiri I'm not looking at a 34 patch set, it's too large.
>
>Break this up into groups of a dozen or so patches each, no
>more. Submit them one at a time and wait for each series
>to be fully reviewed and integrated before moving onto the
>next one.
Yeah. As I stated in the beginning of the cover letter, I did not find a
way to do it. I could split into 2 of 3 patchsets, problem is that I
would introduce interfaces in first patchset that would be only used in
patchset 2 or 3. I believe that is not ok. Do you think that I can do it
like this this time?
Thanks
^ permalink raw reply
* Re: [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2017-10-13 6:26 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, alexander.h.duyck,
john.fastabend, willemb
In-Reply-To: <441b4617-982e-cbc6-0747-026d4a8a0b15@gmail.com>
Thu, Oct 12, 2017 at 11:37:30PM CEST, dsahern@gmail.com wrote:
>On 10/12/17 11:17 AM, Jiri Pirko wrote:
>> So back to the example. First, we create 2 qdiscs. Both will share
>> block number 22. "22" is just an identification. If we don't pass any
>> block number, a new one will be generated by kernel:
>>
>> $ tc qdisc add dev ens7 ingress block 22
>> ^^^^^^^^
>> $ tc qdisc add dev ens8 ingress block 22
>> ^^^^^^^^
>>
>> Now if we list the qdiscs, we will see the block index in the output:
>>
>> $ tc qdisc
>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>
>> Now we can add filter to any of qdiscs sharing the same block:
>>
>> $ tc filter add dev ens7 parent ffff: protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>
>>
>> We will see the same output if we list filters for ens7 and ens8, including stats:
>>
>> $ tc -s filter show dev ens7 ingress
>> filter protocol ip pref 25 flower chain 0
>> filter protocol ip pref 25 flower chain 0 handle 0x1
>> eth_type ipv4
>> dst_ip 192.168.0.0/16
>> not_in_hw
>> action order 1: gact action drop
>> random type none pass val 0
>> index 1 ref 1 bind 1 installed 39 sec used 2 sec
>> Action statistics:
>> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
>> backlog 0b 0p requeues 0
>>
>> $ tc -s filter show dev ens8 ingress
>> filter protocol ip pref 25 flower chain 0
>> filter protocol ip pref 25 flower chain 0 handle 0x1
>> eth_type ipv4
>> dst_ip 192.168.0.0/16
>> not_in_hw
>> action order 1: gact action drop
>> random type none pass val 0
>> index 1 ref 1 bind 1 installed 40 sec used 3 sec
>> Action statistics:
>> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
>> backlog 0b 0p requeues 0
>
>This seems like really odd semantics to me ... a filter added to one
>device shows up on another.
Why is it odd? They share the same block, so it is natural that rule
added to one shows in list of rules for all devices that share the same
block.
>
>Why not make the shared block a standalone object that is configured
>through its own set of commands and then referenced by both devices?
I was thinking about that for a long time. That would require entirely
new set of netlink api and internal kernel handling just for this. Lots
of duplications. The reason is, the current API is strictly build around
ifindex. But the new API would not solve anything. As a user, I still
want so see shared rules in individial device listing, because they
would get processed for the device. So I believe that the proposed
behaviour is correct.
^ permalink raw reply
* Re: [patch net-next 06/34] net: core: use dev->ingress_queue instead of tp->q
From: Jiri Pirko @ 2017-10-13 6:30 UTC (permalink / raw)
To: Daniel Borkmann
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, dsahern,
alexander.h.duyck, john.fastabend, willemb
In-Reply-To: <59DFE287.2040400@iogearbox.net>
Thu, Oct 12, 2017 at 11:45:43PM CEST, daniel@iogearbox.net wrote:
>On 10/12/2017 07:17 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> In sch_handle_egress and sch_handle_ingress, don't use tp->q and use
>> dev->ingress_queue which stores the same pointer instead.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> net/core/dev.c | 21 +++++++++++++++------
>> 1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index fcddccb..cb9e5e5 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3273,14 +3273,18 @@ EXPORT_SYMBOL(dev_loopback_xmit);
>> static struct sk_buff *
>> sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>> {
>> + struct netdev_queue *netdev_queue =
>> + rcu_dereference_bh(dev->ingress_queue);
>> struct tcf_proto *cl = rcu_dereference_bh(dev->egress_cl_list);
>> struct tcf_result cl_res;
>> + struct Qdisc *q;
>>
>> - if (!cl)
>> + if (!cl || !netdev_queue)
>> return skb;
>> + q = netdev_queue->qdisc;
>
>NAK, no additional overhead in the software fast-path of
>sch_handle_{ingress,egress}() like this. There are users out there
>that use tc in software only, so performance is critical here.
Okay, how else do you suggest I can avoid the need to use tp->q?
I was thinking about storing q directly to net_device, which would safe
one dereference, resulting in the same amount as current cl->q.
Thanks.
^ permalink raw reply
* Re: [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances
From: David Miller @ 2017-10-13 6:31 UTC (permalink / raw)
To: jiri
Cc: netdev, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, dsahern,
alexander.h.duyck, john.fastabend, willemb
In-Reply-To: <20171013062101.GA1952@nanopsycho.orion>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 13 Oct 2017 08:21:01 +0200
> Thu, Oct 12, 2017 at 07:21:48PM CEST, davem@davemloft.net wrote:
>>
>>Jiri I'm not looking at a 34 patch set, it's too large.
>>
>>Break this up into groups of a dozen or so patches each, no
>>more. Submit them one at a time and wait for each series
>>to be fully reviewed and integrated before moving onto the
>>next one.
>
> Yeah. As I stated in the beginning of the cover letter, I did not find a
> way to do it. I could split into 2 of 3 patchsets, problem is that I
> would introduce interfaces in first patchset that would be only used in
> patchset 2 or 3. I believe that is not ok. Do you think that I can do it
> like this this time?
Jiri, please try harder.
Thank you.
^ permalink raw reply
* Re: [RFC 1/3] devlink: Add config parameter get/set operations
From: Jiri Pirko @ 2017-10-13 7:04 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Steve Lin, netdev, jiri, davem, michael.chan, linux-pci, linville
In-Reply-To: <20171012181248.GA66136@C02RW35GFVH8.dhcp.broadcom.net>
Thu, Oct 12, 2017 at 08:12:48PM CEST, andrew.gospodarek@broadcom.com wrote:
>On Thu, Oct 12, 2017 at 04:03:17PM +0200, Jiri Pirko wrote:
>> Thu, Oct 12, 2017 at 03:34:20PM CEST, steven.lin1@broadcom.com wrote:
>> >Add support for config parameter get/set commands. Initially used by
>> >bnxt driver, but other drivers can use the same, or new, attributes.
>> >The config_get() and config_set() operations operate as expected, but
>> >note that the driver implementation of the config_set() operation can
>> >indicate whether a restart is necessary for the setting to take
>> >effect.
>> >
>>
>> First of all, I like this approach.
>>
>> I would like to see this patch split into:
>> 1) config-options infrastructure introduction
>> 2) specific config options introductions - would be best to have it
>> per-option. We need to make sure every option is very well described
>> and explained usecases. This is needed in order vendors to share
>> attributes among drivers.
>>
>> More nits inlined.
>>
>>
>> >Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> >Acked-by: Andy Gospodarek <gospo@broadcom.com>
>> >---
>> > include/net/devlink.h | 4 +
>> > include/uapi/linux/devlink.h | 108 ++++++++++++++++++++++
>> > net/core/devlink.c | 207 +++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 319 insertions(+)
>> >
>> > static inline void *devlink_priv(struct devlink *devlink)
>> >diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> >index 0cbca96..e959716 100644
>> >--- a/include/uapi/linux/devlink.h
>> >+++ b/include/uapi/linux/devlink.h
>[...]
>> >@@ -202,6 +267,49 @@ enum devlink_attr {
>> >
>> > DEVLINK_ATTR_ESWITCH_ENCAP_MODE, /* u8 */
>> >
>> >+ /* Configuration Parameters */
>> >+ DEVLINK_ATTR_SRIOV_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_NUM_VF_PER_PF, /* u32 */
>> >+ DEVLINK_ATTR_MAX_NUM_PF_MSIX_VECT, /* u32 */
>> >+ DEVLINK_ATTR_MSIX_VECTORS_PER_VF, /* u32 */
>> >+ DEVLINK_ATTR_NPAR_NUM_PARTITIONS_PER_PORT, /* u32 */
>> >+ DEVLINK_ATTR_NPAR_BW_IN_PERCENT, /* u8 */
>> >+ DEVLINK_ATTR_NPAR_BW_RESERVATION, /* u8 */
>> >+ DEVLINK_ATTR_NPAR_BW_RESERVATION_VALID, /* u8 */
>> >+ DEVLINK_ATTR_NPAR_BW_LIMIT, /* u8 */
>> >+ DEVLINK_ATTR_NPAR_BW_LIMIT_VALID, /* u8 */
>> >+ DEVLINK_ATTR_DCBX_MODE, /* u8 */
>> >+ DEVLINK_ATTR_RDMA_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_MULTIFUNC_MODE, /* u8 */
>> >+ DEVLINK_ATTR_SECURE_NIC_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_IGNORE_ARI_CAPABILITY, /* u8 */
>> >+ DEVLINK_ATTR_LLDP_NEAREST_BRIDGE_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_LLDP_NEAREST_NONTPMR_BRIDGE_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_PME_CAPABILITY_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_MAGIC_PACKET_WOL_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_EEE_PWR_SAVE_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_AUTONEG_PROTOCOL, /* u8 */
>> >+ DEVLINK_ATTR_MEDIA_AUTO_DETECT, /* u8 */
>> >+ DEVLINK_ATTR_PHY_SELECT, /* u8 */
>> >+ DEVLINK_ATTR_PRE_OS_LINK_SPEED_D0, /* u8 */
>> >+ DEVLINK_ATTR_PRE_OS_LINK_SPEED_D3, /* u8 */
>> >+ DEVLINK_ATTR_MBA_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_MBA_BOOT_TYPE, /* u8 */
>> >+ DEVLINK_ATTR_MBA_DELAY_TIME, /* u32 */
>> >+ DEVLINK_ATTR_MBA_SETUP_HOT_KEY, /* u8 */
>> >+ DEVLINK_ATTR_MBA_HIDE_SETUP_PROMPT, /* u8 */
>> >+ DEVLINK_ATTR_MBA_BOOT_RETRY_COUNT, /* u32 */
>> >+ DEVLINK_ATTR_MBA_VLAN_ENABLED, /* u8 */
>> >+ DEVLINK_ATTR_MBA_VLAN_TAG, /* u16 */
>> >+ DEVLINK_ATTR_MBA_BOOT_PROTOCOL, /* u8 */
>> >+ DEVLINK_ATTR_MBA_LINK_SPEED, /* u8 */
>>
>> Okay, I think it is about the time we should start thinking about
>> putting this new config attributes under nester attribute. What do you
>> think?
>>
>
>Steve and I actually had a similar discussion yesterday when I was doing
>a final review of the patches.
>
>My only objection to nesting was coming up with a way to describe these
>functions that made them seem different than existing configuration
>options. In this case of the hardware we are trying to support these
>are all permanent config options, so we would call them
>DEVLINK_ATTR_NVRAM or DEVLINK_ATTR_PERM. Does that seem reasonable to
>others?
The name should go hand-in-hand with the names of the cmds.
^ permalink raw reply
* Re: [RFC 0/3] Adding config get/set to devlink
From: Jiri Pirko @ 2017-10-13 7:08 UTC (permalink / raw)
To: Steve Lin
Cc: Florian Fainelli, David Miller, Roopa Prabhu, netdev, Jiri Pirko,
Michael Chan, linux-pci, John Linville, Andy Gospodarek
In-Reply-To: <CA+Jmh7Em0ZjN0y3FSk1yBhOtPKvFBnJCVA97oZZb7cgXXF_aUA@mail.gmail.com>
Thu, Oct 12, 2017 at 10:12:31PM CEST, steven.lin1@broadcom.com wrote:
>On Thu, Oct 12, 2017 at 3:20 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 10/12/2017 12:06 PM, David Miller wrote:
>>> From: Florian Fainelli <f.fainelli@gmail.com>
>>> Date: Thu, 12 Oct 2017 08:43:59 -0700
>>>
>>>> Once we move ethtool (or however we name its successor) over to
>>>> netlink there is an opportunity for accessing objects that do and do
>>>> not have a netdevice representor today (e.g: management ports on
>>>> switches) with the same interface, and devlink could be used for
>>>> that.
>>>
>>> That is an interesting angle for including this in devlink.
>>>
>>> I'm not so sure what to do about this.
>>>
>>> One suggestion is that devlink is used for getting ethtool stats for
>>> objects lacking netdev representor's, and a new genetlink family is
>>> used for netdev based ethtool.
>>
>> Right, I was also thinking along those lines that we we would have a new
>> generic netlink family for ethtool to support ethtool over netlink.
>>
>>>
>>> I think it's important that we don't expand the scope of devlink
>>> beyond what it was originally designed for.
>>
>> It seems to me like devlink is well defined in what it is not for: it is
>> not meant to be used for any object that is/has a net_device, but it is
>> not well defined for what it can offer to these non network devices. For
>> instance, we have a tremendous amount of operations that are extremely
>> specific to its single user(s) such as mlx5 and mlxsw.
>>
>> For instance, I am not sure how the buffer reservation scheme can be
>> generalized, and this is always the tricky part with a single user
>> facility in that you try to generalize the best you can based on the HW
>> you know. This is not a criticism or meant to be anything negative, this
>> just happens to be the case, and we did not have anything better.
>>
>> So maybe the first thing is to clarify what devlink operations can and
>> should be and what they are absolutely not allowed to cover. We should
>> also clarify whether a generic set/get that Steven is proposing is
>> something that we tolerate, or whether there should be specific function
>> pointers implemented for each attribute, which would be more in line
>> with what has been done thus far.
>
>Hi Florian,
>
>Some of this is subjective, of course, but just to clarify, it did
>seem like implementing a new devlink_op function pointer per attribute
>might be more consistent with what's been done so far. But for code
>reuse purposes - i.e. to avoid replicating essentially the same
>function for each of the 30+ config attributes - I elected to just
>implement a single generic get and set devlink_op.
Also, it this case, unlike any existing cmds, the config options are
all permanent, written in hw. I think it is fine to have one set
of get/set cmd to handle them all at once. Same family.
^ 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