* Re: [PATCH net-next 08/12] net: cls_api: do not expose tcf_block to drivers
From: Jiri Pirko @ 2019-06-21 16:17 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
venkatkumar.duvvuru, cphealy
In-Reply-To: <20190620194917.2298-9-pablo@netfilter.org>
Thu, Jun 20, 2019 at 09:49:13PM CEST, pablo@netfilter.org wrote:
>Expose the block index which is sufficient to look up for the
>tcf_block_cb object.
This patch is not exposing block index. I guess this is a leftover.
^ permalink raw reply
* Re: selftests: bpf: test_libbpf.sh failed at file test_l4lb.o
From: Dan Rue @ 2019-06-21 16:17 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Naresh Kamboju, open list:KERNEL SELFTEST FRAMEWORK, bpf, Netdev,
open list, xdp-newbies, David S. Miller, Daniel Borkmann,
Alexei Starovoitov, Martin Lau, Yonghong Song, john fastabend,
hawk, Jakub Kicinski, Shuah Khan
In-Reply-To: <CAEf4BzbTD8G_zKkj-S3MOeG5Hq3_2zz3bGoXhQtpt0beG8nWJA@mail.gmail.com>
On Thu, Jun 20, 2019 at 10:17:04PM -0700, Andrii Nakryiko wrote:
> On Thu, Jun 20, 2019 at 1:08 AM Naresh Kamboju
> <naresh.kamboju@linaro.org> wrote:
> >
> > selftests: bpf test_libbpf.sh failed running Linux -next kernel
> > 20190618 and 20190619.
> >
> > Here is the log from x86_64,
> > # selftests bpf test_libbpf.sh
> > bpf: test_libbpf.sh_ #
> > # [0] libbpf BTF is required, but is missing or corrupted.
>
> You need at least clang-9.0.0 (not yet released) to run some of these
> tests successfully, as they rely on Clang's support for
> BTF_KIND_VAR/BTF_KIND_DATASEC.
Can there be a runtime check for BTF that emits a skip instead of a fail
in such a case?
Thanks,
Dan
>
> > libbpf: BTF_is #
> > # test_libbpf failed at file test_l4lb.o
> > failed: at_file #
> > # selftests test_libbpf [FAILED]
> > test_libbpf: [FAILED]_ #
> > [FAIL] 29 selftests bpf test_libbpf.sh
> > selftests: bpf_test_libbpf.sh [FAIL]
> >
> > Full test log,
> > https://qa-reports.linaro.org/lkft/linux-next-oe/build/next-20190619/testrun/781777/log
> >
> > Test results comparison,
> > https://qa-reports.linaro.org/lkft/linux-next-oe/tests/kselftest/bpf_test_libbpf.sh
> >
> > Good linux -next tag: next-20190617
> > Bad linux -next tag: next-20190618
> > git branch master
> > git commit 1c6b40509daf5190b1fd2c758649f7df1da4827b
> > git repo
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >
> > Best regards
> > Naresh Kamboju
--
Linaro - Kernel Validation
^ permalink raw reply
* [PATCH net-next] net: macb: use NAPI_POLL_WEIGHT
From: Antoine Tenart @ 2019-06-21 15:28 UTC (permalink / raw)
To: davem, nicolas.ferre
Cc: Antoine Tenart, netdev, linux-kernel, ludovic.desroches,
alexandre.belloni
Use NAPI_POLL_WEIGHT, the default NAPI poll() weight instead of
redefining our own value (which turns out to be 64 as well).
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/cadence/macb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 163deba244ab..1cd1f2c36d6f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3490,7 +3490,7 @@ static int macb_init(struct platform_device *pdev)
queue = &bp->queues[q];
queue->bp = bp;
- netif_napi_add(dev, &queue->napi, macb_poll, 64);
+ netif_napi_add(dev, &queue->napi, macb_poll, NAPI_POLL_WEIGHT);
if (hw_q) {
queue->ISR = GEM_ISR(hw_q - 1);
queue->IER = GEM_IER(hw_q - 1);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates
From: Shuah Khan @ 2019-06-21 16:36 UTC (permalink / raw)
To: Alan Cox, Puranjay Mohan, Bjorn Helgaas, stephen, David S. Miller
Cc: netdev, Linux Kernel Mailing List, linux-kernel-mentees,
linux-pci
In-Reply-To: <20190621162024.53620dd9@alans-desktop>
On 6/21/19 9:20 AM, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
>
>> This patch series removes the private duplicates of PCI definitions in
>> favour of generic definitions defined in pci_regs.h.
>
> Why bother ? It's an ancient obsolete card ?
>
> Do you even have one to test ?
>
>>
>> This driver only uses some of the generic PCI definitons,
>> which are included from pci_regs.h and thier private versions
>> are removed from skfbi.h with all other private defines.
>>
>> The skfbi.h defines PCI_REV_ID and other private defines with different
>> names, these are renamed to Generic PCI names to make them
>> compatible with defines in pci_regs.h.
>>
>> All unused defines are removed from skfbi.h.
>
> I sincerely doubt anyone on the planet is using this card any more.
>
> Alan
>
Thanks Alan!
Stephen Hemminger is suggesting removal as well. Makes sense to me.
David!
What would you recommend the next steps are? Would like driver removed?
thanks,
-- Shuah
^ permalink raw reply
* [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
To: Shuah Khan
Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, linux-pci
This patch series removes the private duplicates of PCI definitions in
favour of generic definitions defined in pci_regs.h.
Puranjay Mohan (3):
net: ethernet: atheros: atlx: Rename local PCI defines to generic
names
net: ethernet: atheros: atlx: Include generic PCI definitions
net: ethernet: atheros: atlx: Remove unused and private PCI
definitions
drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
3 files changed, 3 insertions(+), 5 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
To: Shuah Khan
Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, linux-pci
In-Reply-To: <20190621163921.26188-1-puranjay12@gmail.com>
Rename all local PCI definitons to Generic PCI definitions to make them
compatible with generic definitions present in pci_regs.h
Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl2.c | 4 ++--
drivers/net/ethernet/atheros/atlx/atl2.h | 2 +-
drivers/net/ethernet/atheros/atlx/atlx.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index 3a3fb5ce0fee..478db3fe920a 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -2103,13 +2103,13 @@ static s32 atl2_reset_hw(struct atl2_hw *hw)
int i;
/* Workaround for PCI problem when BIOS sets MMRBC incorrectly. */
- atl2_read_pci_cfg(hw, PCI_REG_COMMAND, &pci_cfg_cmd_word);
+ atl2_read_pci_cfg(hw, PCI_COMMAND, &pci_cfg_cmd_word);
if ((pci_cfg_cmd_word &
(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER)) !=
(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER)) {
pci_cfg_cmd_word |=
(CMD_IO_SPACE|CMD_MEMORY_SPACE|CMD_BUS_MASTER);
- atl2_write_pci_cfg(hw, PCI_REG_COMMAND, &pci_cfg_cmd_word);
+ atl2_write_pci_cfg(hw, PCI_COMMAND, &pci_cfg_cmd_word);
}
/* Clear Interrupt mask to stop board from generating
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
index d97613bd15d5..c53b810a831d 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.h
+++ b/drivers/net/ethernet/atheros/atlx/atl2.h
@@ -202,7 +202,7 @@ static void atl2_force_ps(struct atl2_hw *hw);
#define MII_DBG_DATA 0x1E
/* PCI Command Register Bit Definitions */
-#define PCI_REG_COMMAND 0x04
+#define PCI_COMMAND 0x04
#define CMD_IO_SPACE 0x0001
#define CMD_MEMORY_SPACE 0x0002
#define CMD_BUS_MASTER 0x0004
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.h b/drivers/net/ethernet/atheros/atlx/atlx.h
index 7f5d4e24eb9f..09464cb02ce0 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.h
+++ b/drivers/net/ethernet/atheros/atlx/atlx.h
@@ -445,7 +445,7 @@
#define MII_DBG_DATA 0x1E
/* PCI Command Register Bit Definitions */
-#define PCI_REG_COMMAND 0x04 /* PCI Command Register */
+#define PCI_COMMAND 0x04 /* PCI Command Register */
#define CMD_IO_SPACE 0x0001
#define CMD_MEMORY_SPACE 0x0002
#define CMD_BUS_MASTER 0x0004
--
2.21.0
^ permalink raw reply related
* [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
To: Shuah Khan
Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, linux-pci
In-Reply-To: <20190621163921.26188-1-puranjay12@gmail.com>
Include the uapi/linux/pci_regs.h header file which contains the generic
PCI defines.
Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index 478db3fe920a..58abadd18df2 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -24,6 +24,7 @@
#include <linux/netdevice.h>
#include <linux/pci.h>
#include <linux/pci_ids.h>
+#include <linux/pci_regs.h>
#include <linux/pm.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
--
2.21.0
^ permalink raw reply related
* [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private PCI definitions
From: Puranjay Mohan @ 2019-06-21 16:39 UTC (permalink / raw)
To: Shuah Khan
Cc: Puranjay Mohan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, linux-pci
In-Reply-To: <20190621163921.26188-1-puranjay12@gmail.com>
Remove unused private PCI definitions from skfbi.h because generic PCI
symbols are already included from pci_regs.h.
Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
index c53b810a831d..1b25d6d747de 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.h
+++ b/drivers/net/ethernet/atheros/atlx/atl2.h
@@ -32,7 +32,6 @@
int ethtool_ioctl(struct ifreq *ifr);
#endif
-#define PCI_COMMAND_REGISTER PCI_COMMAND
#define CMD_MEM_WRT_INVALIDATE PCI_COMMAND_INVALIDATE
#define ATL2_WRITE_REG(a, reg, value) (iowrite32((value), \
@@ -202,7 +201,6 @@ static void atl2_force_ps(struct atl2_hw *hw);
#define MII_DBG_DATA 0x1E
/* PCI Command Register Bit Definitions */
-#define PCI_COMMAND 0x04
#define CMD_IO_SPACE 0x0001
#define CMD_MEMORY_SPACE 0x0002
#define CMD_BUS_MASTER 0x0004
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.h b/drivers/net/ethernet/atheros/atlx/atlx.h
index 09464cb02ce0..4d355dbc2d01 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.h
+++ b/drivers/net/ethernet/atheros/atlx/atlx.h
@@ -445,7 +445,6 @@
#define MII_DBG_DATA 0x1E
/* PCI Command Register Bit Definitions */
-#define PCI_COMMAND 0x04 /* PCI Command Register */
#define CMD_IO_SPACE 0x0001
#define CMD_MEMORY_SPACE 0x0002
#define CMD_BUS_MASTER 0x0004
--
2.21.0
^ permalink raw reply related
* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Florian Westphal @ 2019-06-21 16:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, Jakub Sitnicki, netdev, bpf, kernel-team
In-Reply-To: <f373a4d7-c16b-bce2-739d-788525ea4f96@gmail.com>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > AFAICS so far this would be enough:
> >
> > 1. remove the BUG_ON() in skb_orphan, letting it clear skb->sk instead
> > 2. in nf_queue_entry_get_refs(), if skb->sk and no destructor:
> > call nf_tproxy_assign_sock() so a reference gets taken.
> > 3. change skb_steal_sock:
> > static inline struct sock *skb_steal_sock(struct sk_buff *skb, bool *refcounted)
> > [..]
> > *refcounted = skb->destructor != NULL;
> > 4. make tproxy sk assign elide the destructor assigment in case of
> > a listening sk.
> >
>
> Okay, but how do we make sure the skb->sk association does not leak from rcu section ?
From netfilter pov the only escape point is nfqueue (and kfree_skb),
so for tcp/udp it will end up in their respective rx path eventually.
But you are right in that we need to also audit all NF_STOLEN users that
can be invoked from PRE_ROUTING and INPUT hooks.
OUTPUT/FORWARD/POSTROUTING are not relevant, in case skb enters IP forwarding,
it will be dropped there (we have a check to toss skb with socket
attached in forward).
In recent hallway discussion Eric suggested to add a empty destructor
stub, it would allow to do the needed annotation, i.e.
no need to change skb_orphan(), *refcounted would be set via
skb->destructor != noref_listen_skb_destructor check.
> Note we have the noref/refcounted magic for skb_dst(), we might try to use something similar
> for skb->sk
Yes, would be more code churn because we have to replace skb->sk access
by a helper to mask off NOREF bit (or we need to add a "noref" bit in
sk_buff itself).
^ permalink raw reply
* Re: [PATCH net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Neil Horman @ 2019-06-21 16:41 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <CAF=yD-KFZBS7PpvvBkHS5jQdjRr4tWpeHmb7=9QPmvD-RTcpYw@mail.gmail.com>
On Thu, Jun 20, 2019 at 11:16:13AM -0400, Willem de Bruijn wrote:
> On Thu, Jun 20, 2019 at 10:24 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Thu, Jun 20, 2019 at 09:41:30AM -0400, Willem de Bruijn wrote:
> > > On Wed, Jun 19, 2019 at 4:26 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > When an application is run that:
> > > > a) Sets its scheduler to be SCHED_FIFO
> > > > and
> > > > b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> > > > MSG_DONTWAIT flag cleared, its possible for the application to hang
> > > > forever in the kernel. This occurs because when waiting, the code in
> > > > tpacket_snd calls schedule, which under normal circumstances allows
> > > > other tasks to run, including ksoftirqd, which in some cases is
> > > > responsible for freeing the transmitted skb (which in AF_PACKET calls a
> > > > destructor that flips the status bit of the transmitted frame back to
> > > > available, allowing the transmitting task to complete).
> > > >
> > > > However, when the calling application is SCHED_FIFO, its priority is
> > > > such that the schedule call immediately places the task back on the cpu,
> > > > preventing ksoftirqd from freeing the skb, which in turn prevents the
> > > > transmitting task from detecting that the transmission is complete.
> > > >
> > > > We can fix this by converting the schedule call to a completion
> > > > mechanism. By using a completion queue, we force the calling task, when
> > > > it detects there are no more frames to send, to schedule itself off the
> > > > cpu until such time as the last transmitted skb is freed, allowing
> > > > forward progress to be made.
> > > >
> > > > Tested by myself and the reporter, with good results
> > > >
> > > > Appies to the net tree
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > Reported-by: Matteo Croce <mcroce@redhat.com>
> > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > ---
> > >
> > > This is a complex change for a narrow configuration. Isn't a
> > > SCHED_FIFO process preempting ksoftirqd a potential problem for other
> > > networking workloads as well? And the right configuration to always
> > > increase ksoftirqd priority when increasing another process's
> > > priority? Also, even when ksoftirqd kicks in, isn't some progress
> > > still made on the local_bh_enable reached from schedule()?
> > >
> >
> > A few questions here to answer:
>
> Thanks for the detailed explanation.
>
> > Regarding other protocols having this problem, thats not the case, because non
> > packet sockets honor the SK_SNDTIMEO option here (i.e. they sleep for a period
> > of time specified by the SNDTIMEO option if MSG_DONTWAIT isn't set. We could
> > certainly do that, but the current implementation doesn't (opting instead to
> > wait indefinately until the respective packet(s) have transmitted or errored
> > out), and I wanted to maintain that behavior. If there is consensus that packet
> > sockets should honor SNDTIMEO, then I can certainly do that.
> >
> > As for progress made by calling local_bh_enable, My read of the code doesn't
> > have the scheduler calling local_bh_enable at all. Instead schedule uses
> > preempt_disable/preempt_enable_no_resched() to gain exlcusive access to the cpu,
> > which ignores pending softirqs on re-enablement.
>
> Ah, I'm mistaken there, then.
>
> > Perhaps that needs to change,
> > but I'm averse to making scheduler changes for this (the aforementioned concern
> > about complex changes for a narrow use case)
> >
> > Regarding raising the priority of ksoftirqd, that could be a solution, but the
> > priority would need to be raised to a high priority SCHED_FIFO parameter, and
> > that gets back to making complex changes for a narrow problem domain
> >
> > As for the comlexity of the of the solution, I think this is, given your
> > comments the least complex and intrusive change to solve the given problem.
>
> Could it be simpler to ensure do_softirq() gets run here? That would
> allow progress for this case.
>
> > We
> > need to find a way to force the calling task off the cpu while the asynchronous
> > operations in the transmit path complete, and we can do that this way, or by
> > honoring SK_SNDTIMEO. I'm fine with doing the latter, but I didn't want to
> > alter the current protocol behavior without consensus on that.
>
> In general SCHED_FIFO is dangerous with regard to stalling other
> progress, incl. ksoftirqd. But it does appear that this packet socket
> case is special inside networking in calling schedule() directly here.
>
> If converting that, should it convert to logic more akin to other
> sockets, like sock_wait_for_wmem? I haven't had a chance to read up on
> the pros and cons of completion here yet, sorry. Didn't want to delay
> responding until after I get a chance.
>
So, I started looking at implementing SOCK_SNDTIMEO for this patch, and
something occured to me....We still need a mechanism to block in tpacket_snd.
That is to say, other protocol use SK_SNDTIMEO to wait for socket memory to
become available, and that requirement doesn't exist for memory mapped sockets
in AF_PACKET (which tpacket_snd implements the kernel side for). We have memory
mapped frame buffers, which we marshall with an otherwise empty skb, and just
send that (i.e. no waiting on socket memory, we just product an error if we
don't have enough ram to allocate an sk_buff). Given that, we only ever need to
wait for a frame to complete transmission, or get freed in an error path further
down the stack. This probably explains why SK_SNDTIMEO doesn't exist for
AF_PACKET.
Given that, is it worth implementing (as it will just further complicate this
patch, for no additional value), or can we move forward with it as it is?
Neil
^ permalink raw reply
* Re: [PATCH v3 0/3] net: fddi: skfp: Use PCI generic definitions instead of private duplicates
From: Bjorn Helgaas @ 2019-06-21 16:44 UTC (permalink / raw)
To: Alan Cox
Cc: Puranjay Mohan, Shuah Khan, Bjorn Helgaas, netdev,
Linux Kernel Mailing List, linux-kernel-mentees, linux-pci
In-Reply-To: <20190621162024.53620dd9@alans-desktop>
On Fri, Jun 21, 2019 at 04:20:24PM +0100, Alan Cox wrote:
> On Fri, 21 Jun 2019 15:16:04 +0530
> Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> > This patch series removes the private duplicates of PCI definitions in
> > favour of generic definitions defined in pci_regs.h.
>
> Why bother ? It's an ancient obsolete card ?
That's a fair question.
Is there anything that would indicate that "this file is obsolete and
problems shouldn't be fixed"? Nobody wants to waste time on things
that don't need to be fixed, but I don't know how to tell if something
is obsolete.
My naive assumption is that if something is in the tree, it's fair
game for fixes and cleanups.
Bjorn
^ permalink raw reply
* Re: [PATCH net-next 4/6] arm64: dts: fsl: ls1028a: Add Felix switch port DT node
From: Andrew Lunn @ 2019-06-21 16:49 UTC (permalink / raw)
To: Claudiu Manoil
Cc: David S . Miller, devicetree, Alexandre Belloni, netdev,
alexandru.marginean, linux-kernel, UNGLinuxDriver, Allan Nielsen,
Rob Herring, linux-arm-kernel
In-Reply-To: <1561131532-14860-5-git-send-email-claudiu.manoil@nxp.com>
On Fri, Jun 21, 2019 at 06:38:50PM +0300, Claudiu Manoil wrote:
> The switch device features 6 ports, 4 with external links
> and 2 internally facing to the ls1028a SoC and connected via
> fixed links to 2 internal enetc ethernet controller ports.
Hi Claudiu
> + switch@0,5 {
> + compatible = "mscc,felix-switch";
> + reg = <0x000500 0 0 0 0>;
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* external ports */
> + switch_port0: port@0 {
> + reg = <0>;
> + };
> + switch_port1: port@1 {
> + reg = <1>;
> + };
> + switch_port2: port@2 {
> + reg = <2>;
> + };
> + switch_port3: port@3 {
> + reg = <3>;
> + };
> + /* internal to-cpu ports */
> + port@4 {
> + reg = <4>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + port@5 {
> + reg = <5>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
This sounds like a DSA setup, where you have SoC ports connected to
the switch. With DSA, the CPU ports of the switch are special. We
don't create netdev's for them, the binding explicitly list which SoC
interface they are bound to, etc.
What model are you using here? I'm just trying to understand the setup
to ensure it is consistent with the swichdev model.
Thanks
Andrew
^ permalink raw reply
* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Joe Stringer @ 2019-06-21 16:50 UTC (permalink / raw)
To: Jakub Sitnicki; +Cc: Joe Stringer, Florian Westphal, netdev, bpf, kernel-team
In-Reply-To: <CAGn+7TUmgsA8oKw-mM6S5iR4rmNt6sWxjUgw8=qSCHb=m0ROyg@mail.gmail.com>
On Fri, Jun 21, 2019 at 1:44 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> On Fri, Jun 21, 2019, 00:20 Joe Stringer <joe@wand.net.nz> wrote:
>>
>> On Wed, Jun 19, 2019 at 2:14 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>> >
>> > Hey Florian,
>> >
>> > Thanks for taking a look at it.
>> >
>> > On Tue, Jun 18, 2019 at 03:52 PM CEST, Florian Westphal wrote:
>> > > Jakub Sitnicki <jakub@cloudflare.com> wrote:
>> > >> - XDP programs using bpf_sk_lookup helpers, like load balancers, can't
>> > >> find the listening socket to check for SYN cookies with TPROXY redirect.
>> > >
>> > > Sorry for the question, but where is the problem?
>> > > (i.e., is it with TPROXY or bpf side)?
>> >
>> > The way I see it is that the problem is that we have mappings for
>> > steering traffic into sockets split between two places: (1) the socket
>> > lookup tables, and (2) the TPROXY rules.
>> >
>> > BPF programs that need to check if there is a socket the packet is
>> > destined for have access to the socket lookup tables, via the mentioned
>> > bpf_sk_lookup helper, but are unaware of TPROXY redirects.
>> >
>> > For TCP we're able to look up from BPF if there are any established,
>> > request, and "normal" listening sockets. The listening sockets that
>> > receive connections via TPROXY are invisible to BPF progs.
>> >
>> > Why are we interested in finding all listening sockets? To check if any
>> > of them had SYN queue overflow recently and if we should honor SYN
>> > cookies.
>>
>> Why are they invisible? Can't you look them up with bpf_skc_lookup_tcp()?
>
>
> They are invisible in that sense that you can't look them up using the packet 4-tuple. You have to somehow make the XDP/TC progs aware of the TPROXY redirects to find the target sockets.
Isn't that what you're doing in the example from the cover letter
(reincluded below for reference), except with the new program type
rather than XDP/TC progs?
switch (bpf_ntohl(ctx->local_ip4) >> 8) {
case NET1:
ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
ctx->local_port = 81;
return BPF_REDIRECT;
case NET2:
ctx->local_ip4 = bpf_htonl(IP4(127, 0, 0, 1));
ctx->local_port = 82;
return BPF_REDIRECT;
}
That said, I appreciate that even if you find the sockets from XDP,
you'd presumably need some way to retain the socket reference beyond
XDP execution to convince the stack to guide the traffic into that
socket, which would be a whole other effort. For your use case it may
or may not make the most sense.
^ permalink raw reply
* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Paolo Abeni @ 2019-06-21 16:54 UTC (permalink / raw)
To: Florian Westphal, Eric Dumazet; +Cc: Jakub Sitnicki, netdev, bpf, kernel-team
In-Reply-To: <20190621164131.6ghtx6b7dzivsfxk@breakpoint.cc>
Hi,
On Fri, 2019-06-21 at 18:41 +0200, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > AFAICS so far this would be enough:
> > >
> > > 1. remove the BUG_ON() in skb_orphan, letting it clear skb->sk instead
> > > 2. in nf_queue_entry_get_refs(), if skb->sk and no destructor:
> > > call nf_tproxy_assign_sock() so a reference gets taken.
> > > 3. change skb_steal_sock:
> > > static inline struct sock *skb_steal_sock(struct sk_buff *skb, bool *refcounted)
> > > [..]
> > > *refcounted = skb->destructor != NULL;
> > > 4. make tproxy sk assign elide the destructor assigment in case of
> > > a listening sk.
> > >
> >
> > Okay, but how do we make sure the skb->sk association does not leak from rcu section ?
>
> From netfilter pov the only escape point is nfqueue (and kfree_skb),
> so for tcp/udp it will end up in their respective rx path eventually.
> But you are right in that we need to also audit all NF_STOLEN users that
> can be invoked from PRE_ROUTING and INPUT hooks.
>
> OUTPUT/FORWARD/POSTROUTING are not relevant, in case skb enters IP forwarding,
> it will be dropped there (we have a check to toss skb with socket
> attached in forward).
>
> In recent hallway discussion Eric suggested to add a empty destructor
> stub, it would allow to do the needed annotation, i.e.
> no need to change skb_orphan(), *refcounted would be set via
> skb->destructor != noref_listen_skb_destructor check.
Perhaps I'm misreading the above, but it looks like this has some
overlapping with a past attempt:
https://marc.info/?l=linux-netdev&m=150611442802964&w=2
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH net] tcp: refine memory limit test in tcp_fragment()
From: Christoph Paasch @ 2019-06-21 14:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet
In-Reply-To: <20190621130955.147974-1-edumazet@google.com>
> On Jun 21, 2019, at 6:11 AM, Eric Dumazet <edumazet@google.com> wrote:
>
> tcp_fragment() might be called for skbs in the write queue.
>
> Memory limits might have been exceeded because tcp_sendmsg() only
> checks limits at full skb (64KB) boundaries.
>
> Therefore, we need to make sure tcp_fragment() wont punish applications
> that might have setup very low SO_SNDBUF values.
>
> Fixes: f070ef2ac667 ("tcp: tcp_fragment() should apply sane memory limits")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> ---
> net/ipv4/tcp_output.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Tested-by: Christoph Paasch <cpaasch@apple.com>
Thanks!
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 00c01a01b547ec67c971dc25a74c9258563cf871..0ebc33d1c9e5099d163a234930e213ee35e9fbd1 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1296,7 +1296,8 @@ int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
> if (nsize < 0)
> nsize = 0;
>
> - if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf)) {
> + if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf &&
> + tcp_queue != TCP_FRAG_IN_WRITE_QUEUE)) {
> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
> return -ENOMEM;
> }
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
From: Bjorn Helgaas @ 2019-06-21 17:11 UTC (permalink / raw)
To: Puranjay Mohan
Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, Linux PCI
In-Reply-To: <20190621163921.26188-1-puranjay12@gmail.com>
On Fri, Jun 21, 2019 at 11:39 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> This patch series removes the private duplicates of PCI definitions in
> favour of generic definitions defined in pci_regs.h.
>
> Puranjay Mohan (3):
> net: ethernet: atheros: atlx: Rename local PCI defines to generic
> names
> net: ethernet: atheros: atlx: Include generic PCI definitions
> net: ethernet: atheros: atlx: Remove unused and private PCI
> definitions
>
> drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
> drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
> drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
> 3 files changed, 3 insertions(+), 5 deletions(-)
Let's slow this down a little bit; I'm afraid we're going to overwhelm folks.
Before posting more to LKML/netdev, how about we first complete a
sweep of all the drivers to see what we're getting into. It could be
that this will end up being more churn than it's worth. You could
send the result to linux-kernel-mentees, Shuah, me (but not LKML and
netdev), and then we can talk about whether it should be posted all at
once, as several series, etc.
^ permalink raw reply
* Re: [PATCH bpf-next v7 1/9] bpf: implement getsockopt and setsockopt hooks
From: kbuild test robot @ 2019-06-21 17:11 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: kbuild-all, netdev, bpf, davem, ast, daniel, Stanislav Fomichev,
Martin Lau
In-Reply-To: <20190619165957.235580-2-sdf@google.com>
[-- Attachment #1: Type: text/plain, Size: 2828 bytes --]
Hi Stanislav,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Stanislav-Fomichev/bpf-implement-getsockopt-and-setsockopt-hooks/20190621-064924
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm64-defconfig (attached as .config)
compiler: clang version 9.0.0 (git://gitmirror/llvm_project ca42687d62a81fb09fe0187e5f2788077eb6a841)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from net/socket.c:109:
In file included from include/net/busy_poll.h:30:
In file included from include/net/ip.h:32:
In file included from include/net/route.h:28:
In file included from include/net/inetpeer.h:16:
include/net/ipv6.h:378:8: warning: explicitly assigning value of variable of type 'struct ipv6_txoptions *' to itself [-Wself-assign]
opt = (opt);
~~~ ^ ~~~
>> net/socket.c:2130:7: warning: explicitly assigning value of variable of type 'int' to itself [-Wself-assign]
err = (err);
~~~ ^ ~~~
2 warnings generated.
vim +/int +2130 net/socket.c
2100
2101 /*
2102 * Get a socket option. Because we don't know the option lengths we have
2103 * to pass a user mode parameter for the protocols to sort out.
2104 */
2105
2106 static int __sys_getsockopt(int fd, int level, int optname,
2107 char __user *optval, int __user *optlen)
2108 {
2109 int err, fput_needed;
2110 struct socket *sock;
2111 int max_optlen;
2112
2113 sock = sockfd_lookup_light(fd, &err, &fput_needed);
2114 if (sock != NULL) {
2115 err = security_socket_getsockopt(sock, level, optname);
2116 if (err)
2117 goto out_put;
2118
2119 max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
2120
2121 if (level == SOL_SOCKET)
2122 err =
2123 sock_getsockopt(sock, level, optname, optval,
2124 optlen);
2125 else
2126 err =
2127 sock->ops->getsockopt(sock, level, optname, optval,
2128 optlen);
2129
> 2130 err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level, optname,
2131 optval, optlen,
2132 max_optlen, err);
2133 out_put:
2134 fput_light(sock->file, fput_needed);
2135 }
2136 return err;
2137 }
2138
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44368 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private PCI definitions
From: Bjorn Helgaas @ 2019-06-21 17:13 UTC (permalink / raw)
To: Puranjay Mohan
Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, Linux PCI
In-Reply-To: <20190621163921.26188-4-puranjay12@gmail.com>
On Fri, Jun 21, 2019 at 11:40 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
>
> Remove unused private PCI definitions from skfbi.h because generic PCI
> symbols are already included from pci_regs.h.
>
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> ---
> drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
> drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
> 2 files changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/atheros/atlx/atl2.h b/drivers/net/ethernet/atheros/atlx/atl2.h
> index c53b810a831d..1b25d6d747de 100644
> --- a/drivers/net/ethernet/atheros/atlx/atl2.h
> +++ b/drivers/net/ethernet/atheros/atlx/atl2.h
> @@ -32,7 +32,6 @@
> int ethtool_ioctl(struct ifreq *ifr);
> #endif
>
> -#define PCI_COMMAND_REGISTER PCI_COMMAND
> #define CMD_MEM_WRT_INVALIDATE PCI_COMMAND_INVALIDATE
>
> #define ATL2_WRITE_REG(a, reg, value) (iowrite32((value), \
> @@ -202,7 +201,6 @@ static void atl2_force_ps(struct atl2_hw *hw);
> #define MII_DBG_DATA 0x1E
>
> /* PCI Command Register Bit Definitions */
> -#define PCI_COMMAND 0x04
> #define CMD_IO_SPACE 0x0001
> #define CMD_MEMORY_SPACE 0x0002
> #define CMD_BUS_MASTER 0x0004
These bit definitions (CMD_IO_SPACE, CMD_MEMORY_SPACE, etc) are also
generic PCI things that should be replaced with PCI_COMMAND_IO,
PCI_COMMAND_MEMORY, etc. I haven't looked at the file, but there are
likely more.
^ permalink raw reply
* Re: [PATCH net-next 04/12] net: sched: add tcf_block_setup()
From: Jiri Pirko @ 2019-06-21 17:16 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
venkatkumar.duvvuru, cphealy
In-Reply-To: <20190620194917.2298-5-pablo@netfilter.org>
Thu, Jun 20, 2019 at 09:49:09PM CEST, pablo@netfilter.org wrote:
[...]
>
>+static LIST_HEAD(tcf_block_cb_list);
I still don't like the global list. Have to go throught the code more
carefully, but why you can't pass the priv/ctx from tc/netfilter. From
tc it would be tcf_block as it is now, from netfilter something else.
^ permalink raw reply
* Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
From: Joe Perches @ 2019-06-21 17:27 UTC (permalink / raw)
To: Bjorn Helgaas, Puranjay Mohan, Jay Cliburn, Chris Snook
Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
linux-kernel-mentees, Linux PCI
In-Reply-To: <CAErSpo5TMPokae7BMY8ZcOXtW=GeGsWXX_bqS8SrZnh0pEQYxw@mail.gmail.com>
(adding the atlx maintainers to cc)
On Fri, 2019-06-21 at 12:11 -0500, Bjorn Helgaas wrote:
> On Fri, Jun 21, 2019 at 11:39 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > This patch series removes the private duplicates of PCI definitions in
> > favour of generic definitions defined in pci_regs.h.
> >
> > Puranjay Mohan (3):
> > net: ethernet: atheros: atlx: Rename local PCI defines to generic
> > names
> > net: ethernet: atheros: atlx: Include generic PCI definitions
> > net: ethernet: atheros: atlx: Remove unused and private PCI
> > definitions
> >
> > drivers/net/ethernet/atheros/atlx/atl2.c | 5 +++--
> > drivers/net/ethernet/atheros/atlx/atl2.h | 2 --
> > drivers/net/ethernet/atheros/atlx/atlx.h | 1 -
> > 3 files changed, 3 insertions(+), 5 deletions(-)
>
> Let's slow this down a little bit; I'm afraid we're going to overwhelm folks.
I generally disagree.
Consolidation of these sorts of changes are generally
better done treewide all at once, posted as a series to
a list and maintainers allowing time (weeks to months)
for the specific maintainers to accept them and then
whatever remainder exists reposted and possibly applied
by an overall maintainer (e.g.: Dave M)
> Before posting more to LKML/netdev, how about we first complete a
> sweep of all the drivers to see what we're getting into. It could be
> that this will end up being more churn than it's worth.
Also doubtful.
Subsystem specific local PCI #defines without generic
naming is poor style and makes treewide grep and
refactoring much more difficult.
The atlx maintainers should definitely have been cc'd
on these patches.
Jay Cliburn <jcliburn@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)
Chris Snook <chris.snook@gmail.com> (maintainer:ATLX ETHERNET DRIVERS)
Puranjay, can you please do a few things more here:
1: Make sure you use scripts/get_maintainer.pl to cc the
appropriate people.
2: Show that you compiled the object files and verified
where possible that there are no object file changes.
3: State that there are no object changes in the proposed
commit log.
thanks.
^ permalink raw reply
* [PATCH net-next v5 0/7] net/sched: Add txtime-assist support for taprio.
From: Vedang Patel @ 2019-06-21 17:28 UTC (permalink / raw)
To: netdev
Cc: jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, jakub.kicinski, m-karicheri2,
sergei.shtylyov, eric.dumazet, aaron.f.brown, Vedang Patel
Changes in v5:
- Commit message improved for the igb patch (patch #1).
- Fixed typo in commit message for etf patch (patch #2).
Changes in v4:
- Remove inline directive from functions in foo.c.
- Fix spacing in pkt_sched.h (for etf patch).
Changes in v3:
- Simplify implementation for taprio flags.
- txtime_delay can only be set if txtime-assist mode is enabled.
- txtime_delay and flags will only be visible in tc output if set by user.
- Minor changes in error reporting.
Changes in v2:
- Txtime-offload has now been renamed to txtime-assist mode.
- Renamed the offload parameter to flags.
- Removed the code which introduced the hardware offloading functionality.
Original Cover letter (with above changes included)
--------------------------------------------------
Currently, we are seeing packets being transmitted outside their
timeslices. We can confirm that the packets are being dequeued at the right
time. So, the delay is induced after the packet is dequeued, because
taprio, without any offloading, has no control of when a packet is actually
transmitted.
In order to solve this, we are making use of the txtime feature provided by
ETF qdisc. Hardware offloading needs to be supported by the ETF qdisc in
order to take advantage of this feature. The taprio qdisc will assign
txtime (in skb->tstamp) for all the packets which do not have the txtime
allocated via the SO_TXTIME socket option. For the packets which already
have SO_TXTIME set, taprio will validate whether the packet will be
transmitted in the correct interval.
In order to support this, the following parameters have been added:
- flags (taprio): This is added in order to support different offloading
modes which will be added in the future.
- txtime-delay (taprio): This indicates the minimum time it will take for
the packet to hit the wire after it reaches taprio_enqueue(). This is
useful in determining whether we can transmit the packet in the remaining
time if the gate corresponding to the packet is currently open.
- skip_skb_check (ETF): ETF currently drops any packet which does not have
the SO_TXTIME socket option set. This check can be skipped by specifying
this option.
Following is an example configuration:
tc qdisc replace dev $IFACE parent root handle 100 taprio \\
num_tc 3 \\
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \\
queues 1@0 1@0 1@0 \\
base-time $BASE_TIME \\
sched-entry S 01 300000 \\
sched-entry S 02 300000 \\
sched-entry S 04 400000 \\
flags 0x1 \\
txtime-delay 200000 \\
clockid CLOCK_TAI
tc qdisc replace dev $IFACE parent 100:1 etf \\
offload delta 200000 clockid CLOCK_TAI skip_skb_check
Here, the "flags" parameter is indicating that the txtime-assist mode is
enabled. Also, all the traffic classes have been assigned the same queue.
This is to prevent the traffic classes in the lower priority queues from
getting starved. Note that this configuration is specific to the i210
ethernet card. Other network cards where the hardware queues are given the
same priority, might be able to utilize more than one queue.
Following are some of the other highlights of the series:
- Fix a bug where hardware timestamping and SO_TXTIME options cannot be
used together. (Patch 1)
- Introduces the skip_skb_check option. (Patch 2)
- Make TxTime assist mode work with TCP packets (Patch 7).
The following changes are recommended to be done in order to get the best
performance from taprio in this mode:
# TSN in general does not allow Jumbo frames.
ip link set dev enp1s0 mtu 1514
# Disable segmentation offload. This is to prevent NIC from sending packets
# after the gate for a traffic class has closed.
ethtool -K eth0 gso off
ethtool -K eth0 tso off
# Disable energy efficient ethernet to make sure there are no latency
# spikes when NIC is trying to wake up when the packet is supposed to be
# sent.
ethtool --set-eee eth0 eee off
Thanks,
Vedang Patel
Vedang Patel (7):
igb: clear out skb->tstamp after reading the txtime
etf: Add skip_sock_check
taprio: calculate cycle_time when schedule is installed
taprio: Remove inline directive
taprio: Add support for txtime-assist mode
taprio: make clock reference conversions easier
taprio: Adjust timestamps for TCP packets
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
include/uapi/linux/pkt_sched.h | 5 +
net/sched/sch_etf.c | 10 +
net/sched/sch_taprio.c | 431 +++++++++++++++++++++++++++---
4 files changed, 413 insertions(+), 34 deletions(-)
--
2.7.3
^ permalink raw reply
* [PATCH net-next v5 1/7] igb: clear out skb->tstamp after reading the txtime
From: Vedang Patel @ 2019-06-21 17:28 UTC (permalink / raw)
To: netdev
Cc: jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, jakub.kicinski, m-karicheri2,
sergei.shtylyov, eric.dumazet, aaron.f.brown, Vedang Patel
In-Reply-To: <1561138108-12943-1-git-send-email-vedang.patel@intel.com>
If a packet which is utilizing the launchtime feature (via SO_TXTIME socket
option) also requests the hardware transmit timestamp, the hardware
timestamp is not delivered to the userspace. This is because the value in
skb->tstamp is mistaken as the software timestamp.
Applications, like ptp4l, request a hardware timestamp by setting the
SOF_TIMESTAMPING_TX_HARDWARE socket option. Whenever a new timestamp is
detected by the driver (this work is done in igb_ptp_tx_work() which calls
igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp in the
ERR_QUEUE for the userspace to read. When the userspace is ready, it will
issue a recvmsg() call to collect this timestamp. The problem is in this
recvmsg() call. If the skb->tstamp is not cleared out, it will be
interpreted as a software timestamp and the hardware tx timestamp will not
be successfully sent to the userspace. Look at skb_is_swtx_tstamp() and the
callee function __sock_recv_timestamp() in net/socket.c for more details.
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index fc925adbd9fa..f66dae72fe37 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5688,6 +5688,7 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring,
*/
if (tx_ring->launchtime_enable) {
ts = ns_to_timespec64(first->skb->tstamp);
+ first->skb->tstamp = 0;
context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32);
} else {
context_desc->seqnum_seed = 0;
--
2.7.3
^ permalink raw reply related
* [PATCH net-next v5 2/7] etf: Add skip_sock_check
From: Vedang Patel @ 2019-06-21 17:28 UTC (permalink / raw)
To: netdev
Cc: jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, jakub.kicinski, m-karicheri2,
sergei.shtylyov, eric.dumazet, aaron.f.brown, Vedang Patel
In-Reply-To: <1561138108-12943-1-git-send-email-vedang.patel@intel.com>
Currently, etf expects a socket with SO_TXTIME option set for each packet
it encounters. So, it will drop all other packets. But, in the future
commits we are planning to add functionality where tstamp value will be set
by another qdisc. Also, some packets which are generated from within the
kernel (e.g. ICMP packets) do not have any socket associated with them.
So, this commit adds support for skip_sock_check. When this option is set,
etf will skip checking for a socket and other associated options for all
skbs.
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
include/uapi/linux/pkt_sched.h | 1 +
net/sched/sch_etf.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 8b2f993cbb77..409d1616472d 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -990,6 +990,7 @@ struct tc_etf_qopt {
__u32 flags;
#define TC_ETF_DEADLINE_MODE_ON BIT(0)
#define TC_ETF_OFFLOAD_ON BIT(1)
+#define TC_ETF_SKIP_SOCK_CHECK BIT(2)
};
enum {
diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
index db0c2ba1d156..cebfb65d8556 100644
--- a/net/sched/sch_etf.c
+++ b/net/sched/sch_etf.c
@@ -22,10 +22,12 @@
#define DEADLINE_MODE_IS_ON(x) ((x)->flags & TC_ETF_DEADLINE_MODE_ON)
#define OFFLOAD_IS_ON(x) ((x)->flags & TC_ETF_OFFLOAD_ON)
+#define SKIP_SOCK_CHECK_IS_SET(x) ((x)->flags & TC_ETF_SKIP_SOCK_CHECK)
struct etf_sched_data {
bool offload;
bool deadline_mode;
+ bool skip_sock_check;
int clockid;
int queue;
s32 delta; /* in ns */
@@ -77,6 +79,9 @@ static bool is_packet_valid(struct Qdisc *sch, struct sk_buff *nskb)
struct sock *sk = nskb->sk;
ktime_t now;
+ if (q->skip_sock_check)
+ goto skip;
+
if (!sk)
return false;
@@ -92,6 +97,7 @@ static bool is_packet_valid(struct Qdisc *sch, struct sk_buff *nskb)
if (sk->sk_txtime_deadline_mode != q->deadline_mode)
return false;
+skip:
now = q->get_time();
if (ktime_before(txtime, now) || ktime_before(txtime, q->last))
return false;
@@ -385,6 +391,7 @@ static int etf_init(struct Qdisc *sch, struct nlattr *opt,
q->clockid = qopt->clockid;
q->offload = OFFLOAD_IS_ON(qopt);
q->deadline_mode = DEADLINE_MODE_IS_ON(qopt);
+ q->skip_sock_check = SKIP_SOCK_CHECK_IS_SET(qopt);
switch (q->clockid) {
case CLOCK_REALTIME:
@@ -473,6 +480,9 @@ static int etf_dump(struct Qdisc *sch, struct sk_buff *skb)
if (q->deadline_mode)
opt.flags |= TC_ETF_DEADLINE_MODE_ON;
+ if (q->skip_sock_check)
+ opt.flags |= TC_ETF_SKIP_SOCK_CHECK;
+
if (nla_put(skb, TCA_ETF_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
--
2.7.3
^ permalink raw reply related
* [PATCH net-next v5 3/7] taprio: calculate cycle_time when schedule is installed
From: Vedang Patel @ 2019-06-21 17:28 UTC (permalink / raw)
To: netdev
Cc: jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, jakub.kicinski, m-karicheri2,
sergei.shtylyov, eric.dumazet, aaron.f.brown, Vedang Patel
In-Reply-To: <1561138108-12943-1-git-send-email-vedang.patel@intel.com>
cycle time for a particular schedule is calculated only when it is first
installed. So, it makes sense to just calculate it once right after the
'cycle_time' parameter has been parsed and store it in cycle_time.
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
net/sched/sch_taprio.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 9ecfb8f5902a..a41d7d4434ee 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -108,22 +108,6 @@ static void switch_schedules(struct taprio_sched *q,
*admin = NULL;
}
-static ktime_t get_cycle_time(struct sched_gate_list *sched)
-{
- struct sched_entry *entry;
- ktime_t cycle = 0;
-
- if (sched->cycle_time != 0)
- return sched->cycle_time;
-
- list_for_each_entry(entry, &sched->entries, list)
- cycle = ktime_add_ns(cycle, entry->interval);
-
- sched->cycle_time = cycle;
-
- return cycle;
-}
-
static int taprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
struct sk_buff **to_free)
{
@@ -524,6 +508,15 @@ static int parse_taprio_schedule(struct nlattr **tb,
if (err < 0)
return err;
+ if (!new->cycle_time) {
+ struct sched_entry *entry;
+ ktime_t cycle = 0;
+
+ list_for_each_entry(entry, &new->entries, list)
+ cycle = ktime_add_ns(cycle, entry->interval);
+ new->cycle_time = cycle;
+ }
+
return 0;
}
@@ -605,7 +598,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
return 0;
}
- cycle = get_cycle_time(sched);
+ cycle = sched->cycle_time;
/* The qdisc is expected to have at least one sched_entry. Moreover,
* any entry must have 'interval' > 0. Thus if the cycle time is zero,
@@ -632,7 +625,7 @@ static void setup_first_close_time(struct taprio_sched *q,
first = list_first_entry(&sched->entries,
struct sched_entry, list);
- cycle = get_cycle_time(sched);
+ cycle = sched->cycle_time;
/* FIXME: find a better place to do this */
sched->cycle_close_time = ktime_add_ns(base, cycle);
--
2.7.3
^ permalink raw reply related
* [PATCH net-next v5 6/7] taprio: make clock reference conversions easier
From: Vedang Patel @ 2019-06-21 17:28 UTC (permalink / raw)
To: netdev
Cc: jeffrey.t.kirsher, davem, jhs, xiyou.wangcong, jiri,
intel-wired-lan, vinicius.gomes, l, jakub.kicinski, m-karicheri2,
sergei.shtylyov, eric.dumazet, aaron.f.brown, Vedang Patel
In-Reply-To: <1561138108-12943-1-git-send-email-vedang.patel@intel.com>
Later in this series we will need to transform from
CLOCK_MONOTONIC (used in TCP) to the clock reference used in TAPRIO.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
net/sched/sch_taprio.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 6911f22fd8dc..44540c30887e 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -61,6 +61,7 @@ struct taprio_sched {
struct Qdisc **qdiscs;
struct Qdisc *root;
u32 flags;
+ enum tk_offsets tk_offset;
int clockid;
atomic64_t picos_per_byte; /* Using picoseconds because for 10Gbps+
* speeds it's sub-nanoseconds per byte
@@ -71,7 +72,6 @@ struct taprio_sched {
struct sched_entry __rcu *current_entry;
struct sched_gate_list __rcu *oper_sched;
struct sched_gate_list __rcu *admin_sched;
- ktime_t (*get_time)(void);
struct hrtimer advance_timer;
struct list_head taprio_list;
int txtime_delay;
@@ -85,6 +85,20 @@ static ktime_t sched_base_time(const struct sched_gate_list *sched)
return ns_to_ktime(sched->base_time);
}
+static ktime_t taprio_get_time(struct taprio_sched *q)
+{
+ ktime_t mono = ktime_get();
+
+ switch (q->tk_offset) {
+ case TK_OFFS_MAX:
+ return mono;
+ default:
+ return ktime_mono_to_any(mono, q->tk_offset);
+ }
+
+ return KTIME_MAX;
+}
+
static void taprio_free_sched_cb(struct rcu_head *head)
{
struct sched_gate_list *sched = container_of(head, struct sched_gate_list, rcu);
@@ -288,7 +302,7 @@ static long get_packet_txtime(struct sk_buff *skb, struct Qdisc *sch)
struct sched_entry *entry;
bool sched_changed;
- now = q->get_time();
+ now = taprio_get_time(q);
minimum_time = ktime_add_ns(now, q->txtime_delay);
rcu_read_lock();
@@ -479,7 +493,7 @@ static struct sk_buff *taprio_dequeue(struct Qdisc *sch)
continue;
len = qdisc_pkt_len(skb);
- guard = ktime_add_ns(q->get_time(),
+ guard = ktime_add_ns(taprio_get_time(q),
length_to_duration(q, len));
/* In the case that there's no gate entry, there's no
@@ -848,7 +862,7 @@ static int taprio_get_start_time(struct Qdisc *sch,
s64 n;
base = sched_base_time(sched);
- now = q->get_time();
+ now = taprio_get_time(q);
if (ktime_after(base, now)) {
*start = base;
@@ -1094,16 +1108,16 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
switch (q->clockid) {
case CLOCK_REALTIME:
- q->get_time = ktime_get_real;
+ q->tk_offset = TK_OFFS_REAL;
break;
case CLOCK_MONOTONIC:
- q->get_time = ktime_get;
+ q->tk_offset = TK_OFFS_MAX;
break;
case CLOCK_BOOTTIME:
- q->get_time = ktime_get_boottime;
+ q->tk_offset = TK_OFFS_BOOT;
break;
case CLOCK_TAI:
- q->get_time = ktime_get_clocktai;
+ q->tk_offset = TK_OFFS_TAI;
break;
default:
NL_SET_ERR_MSG(extack, "Invalid 'clockid'");
--
2.7.3
^ permalink raw reply related
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