* XDP_DROP and XDP_TX (Was: Re: [net-next PATCH v5 0/6] XDP for virtio_net)
From: Jesper Dangaard Brouer @ 2016-12-13 8:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Miller, john.fastabend, daniel, mst, shm, tgraf,
john.r.fastabend, netdev, brouer
In-Reply-To: <20161208193814.GA1954@ast-mbp.thefacebook.com>
On Thu, 8 Dec 2016 11:38:16 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Thu, Dec 08, 2016 at 02:17:02PM -0500, David Miller wrote:
> > From: John Fastabend <john.fastabend@gmail.com>
> > Date: Wed, 07 Dec 2016 12:10:47 -0800
> >
[...]
> > > Can't we disable XDP_TX somehow? Many people might only want RX drop,
> > > and extra queues are not always there.
> > >
> >
> > Alexei, Daniel, any thoughts on this?
>
> I don't like it.
I don't know that the use-case virtio XDP_TX is, but I still think we
should implement it for this virtio_net driver, as it allow easier
testing of XDP programs without physical HW.
BUT I do believe XDP_DROP and XDP_TX should be two different capabilities.
I can easily imagine that an older driver only wants to implement the
XDP_DROP facility. The reason is that XDP_TX would require changing too
much driver code, which is a concern for an old, stable and time-proven
driver.
I can also imagine wanting to implement XDP_DROP on my OpenWRT home
router, which I don't think can get an extra HW TX queue for XDP_TX.
I even have a practical use-case for my OpenWRT home router. I
experience a Windows machine being connected to my home network, it
caused some (WiFi) traffic storm that overloaded the small OpenWRT box,
so much that it couldn't even route packets for other machines
connected (on Ethernet). This could also be an IoT device or an
infected machine participating in a DDoS attack. I fairly quickly
identified the bad machine and disconnected it from the network, but
for e.g. conference or hotel networks is can be harder to identify and
disconnect offenders. An XDP eBPF (ddos) filter could allow the sysadm
to "virtually" disconnect a certain MAC address, and restore operation.
> > I know we were trying to claim some base level of feature support for
> > all XDP drivers. I am sympathetic to this argument though for DDOS we
> > do not need XDP_TX support. And virtio can become queue constrained
> > in some cases.
>
> Without XDP_TX it's too crippled. adjust_head() won't be possible,
> packet mangling would have to be disabled and so on.
Even without XDP_TX support, adjust_head() must be supported. The
XDP_PASS action still requires ability to modify the packet.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: stmmac DT property snps,axi_all
From: Giuseppe CAVALLARO @ 2016-12-13 8:46 UTC (permalink / raw)
To: Niklas Cassel, Alexandre Torgue; +Cc: netdev
In-Reply-To: <548b2225-a395-ca52-15cf-ee4f3dd5cd93@axis.com>
On 12/13/2016 9:26 AM, Niklas Cassel wrote:
> On 12/13/2016 07:47 AM, Giuseppe CAVALLARO wrote:
>> Hello Niklas, Alex,
>>
>> my fault and a step behind... Current code is OK
>> when manage the AAL that, although it is passed from
>> the axi structure, it is always used to program,
>> for all the chip versions, the writable bit inside the
>> DMA_BUS_MODE register.
>> So I guess no extra patch is needed.
>
> Hello Giuseppe
>
> I'm not sure what you mean.
>
> Yes, when setting "snps,aal",
> the code is correct for all versions of the IP.
>
> However, "snps,axi_all" seems to just be dead code.
> Try git grep in the whole tree.
> So I still think that my patch is valid.
ok there is the snps,aal, proceed with your patch and
add my Acked-by. Sorry for this traffic but I am trying
to keep myself aligned with all these new patches.
So, IIUC, you have some new patch for TX Timer; pls
do not forget to add me on copy.
Regards
Peppe
>
>
>>
>> Regards
>> Peppe
>>
>> On 12/12/2016 3:18 PM, Giuseppe CAVALLARO wrote:
>>> Please Niklas
>>>
>>> when you send the patch, add my
>>>
>>> Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>
>>>
>>> On 12/9/2016 10:53 AM, Niklas Cassel wrote:
>>>> On 12/09/2016 10:20 AM, Niklas Cassel wrote:
>>>>> On 12/08/2016 02:36 PM, Alexandre Torgue wrote:
>>>>>> Hi Niklas,
>>>>>>
>>>>>> On 12/05/2016 05:18 PM, Niklas Cassel wrote:
>>>>>>> Hello Giuseppe
>>>>>>>
>>>>>>>
>>>>>>> I'm trying to figure out what snps,axi_all is supposed to represent.
>>>>>>>
>>>>>>> It appears that the value is saved, but never used in the code.
>>>>>>>
>>>>>>> Looking at the register specification, I'm guessing that it represents
>>>>>>> Address-Aligned Beats, but there is already the property snps,aal
>>>>>>> for that.
>>>>>> IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus
>>>>>> mode register) and reflects the aal bit in DMA bus register.
>>>>>> As you know we use "snps,aal" to set aal bit in DMA bus register.
>>>>>> So "snps,axi_all" entry seems useless. Let's see with Peppe.
>>>>> Ok, I see. GMAC and GMAC4 is different here.
>>>>>
>>>>> For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
>>>>> It's not reflected anywhere else.
>>>>>
>>>>> The code is correct in the driver.
>>>>>
>>>>> If snps,axi_all is just created for a read-only register,
>>>>> and it is currently never used in the code,
>>>>> while we have snps,aal, which is correct and works,
>>>>> I guess it should be ok to remove snps,axi_all.
>>>>>
>>>>> I can cook up a patch.
>>>>>
>>>>
>>>> Here we go :)
>>>>
>>>> I will send it as a real patch once net-next reopens.
>>>>
>>>>
>>>>> From defc01cb7c22611b89d9cf1fcae72544092bd62c Mon Sep 17 00:00:00 2001
>>>> From: Niklas Cassel <niklas.cassel@axis.com>
>>>> Date: Fri, 9 Dec 2016 10:27:00 +0100
>>>> Subject: [PATCH net-next] net: stmmac: remove unused duplicate property
>>>> snps,axi_all
>>>>
>>>> For core revision 3.x Address-Aligned Beats is available in two
>>>> registers.
>>>> The DT property snps,aal was created for AAL in the DMA bus register,
>>>> which is a read/write bit.
>>>> The DT property snps,axi_all was created for AXI_AAL in the AXI bus mode
>>>> register, which is a read only bit that reflects the value of AAL in the
>>>> DMA bus register.
>>>>
>>>> Since the value of snps,axi_all is never used in the driver,
>>>> and since the property was created for a bit that is read only,
>>>> it should be safe to remove the property.
>>>>
>>>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>>>> ---
>>>> Documentation/devicetree/bindings/net/stmmac.txt | 1 -
>>>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
>>>> include/linux/stmmac.h | 1 -
>>>> 3 files changed, 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt
>>>> b/Documentation/devicetree/bindings/net/stmmac.txt
>>>> index 128da752fec9..c3d2fd480a1b 100644
>>>> --- a/Documentation/devicetree/bindings/net/stmmac.txt
>>>> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
>>>> @@ -65,7 +65,6 @@ Optional properties:
>>>> - snps,wr_osr_lmt: max write outstanding req. limit
>>>> - snps,rd_osr_lmt: max read outstanding req. limit
>>>> - snps,kbbe: do not cross 1KiB boundary.
>>>> - - snps,axi_all: align address
>>>> - snps,blen: this is a vector of supported burst length.
>>>> - snps,fb: fixed-burst
>>>> - snps,mb: mixed-burst
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>>> index 082cd48db6a7..60ba8993c650 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>>> @@ -121,7 +121,6 @@ static struct stmmac_axi *stmmac_axi_setup(struct
>>>> platform_device *pdev)
>>>> axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
>>>> axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
>>>> axi->axi_kbbe = of_property_read_bool(np, "snps,axi_kbbe");
>>>> - axi->axi_axi_all = of_property_read_bool(np, "snps,axi_all");
>>>> axi->axi_fb = of_property_read_bool(np, "snps,axi_fb");
>>>> axi->axi_mb = of_property_read_bool(np, "snps,axi_mb");
>>>> axi->axi_rb = of_property_read_bool(np, "snps,axi_rb");
>>>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>>>> index 266dab9ad782..889e0e9a3f1c 100644
>>>> --- a/include/linux/stmmac.h
>>>> +++ b/include/linux/stmmac.h
>>>> @@ -103,7 +103,6 @@ struct stmmac_axi {
>>>> u32 axi_wr_osr_lmt;
>>>> u32 axi_rd_osr_lmt;
>>>> bool axi_kbbe;
>>>> - bool axi_axi_all;
>>>> u32 axi_blen[AXI_BLEN];
>>>> bool axi_fb;
>>>> bool axi_mb;
>>>>
>>>
>>>
>>
>
>
^ permalink raw reply
* [PATCH iproute2 2/2] tc/m_tunnel_key: Add dest UDP port to tunnel key action
From: Hadar Hen Zion @ 2016-12-13 8:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Or Gerlitz, Roi Dayan, Amir Vadai, Hadar Hen Zion
In-Reply-To: <1481616467-769-1-git-send-email-hadarh@mellanox.com>
Enhance tunnel key action parameters by adding destination UDP port.
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
man/man8/tc-tunnel_key.8 | 6 ++++++
tc/m_tunnel_key.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/man/man8/tc-tunnel_key.8 b/man/man8/tc-tunnel_key.8
index 17b15b9..2e56973 100644
--- a/man/man8/tc-tunnel_key.8
+++ b/man/man8/tc-tunnel_key.8
@@ -15,6 +15,7 @@ tunnel_key - Tunnel metadata manipulation
.BR dst_ip
.IR ADDRESS
.BI id " KEY_ID"
+.BI dst_port " UDP_PORT"
.SH DESCRIPTION
The
@@ -61,6 +62,8 @@ Set tunnel metadata to be used by the IP tunnel device. Requires
and
.B dst_ip
options.
+.B dst_port
+is optional.
.RS
.TP
.B id
@@ -71,6 +74,9 @@ Outer header source IP address (IPv4 or IPv6)
.TP
.B dst_ip
Outer header destination IP address (IPv4 or IPv6)
+.TP
+.B dst_port
+Outer header destination UDP port
.RE
.SH EXAMPLES
The following example encapsulates incoming ICMP packets on eth0 into a vxlan
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index f4a20e2..58a3042 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -60,6 +60,20 @@ static int tunnel_key_parse_key_id(const char *str, int type,
return ret;
}
+static int tunnel_key_parse_dst_port(char *str, int type, struct nlmsghdr *n)
+{
+ int ret;
+ __be16 dst_port;
+
+ ret = get_be16(&dst_port, str, 10);
+ if (ret)
+ return -1;
+
+ addattr16(n, MAX_MSG, type, dst_port);
+
+ return 0;
+}
+
static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
int tca_id, struct nlmsghdr *n)
{
@@ -128,6 +142,14 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
return -1;
}
has_key_id = 1;
+ } else if (matches(*argv, "dst_port") == 0) {
+ NEXT_ARG();
+ ret = tunnel_key_parse_dst_port(*argv,
+ TCA_TUNNEL_KEY_ENC_DST_PORT, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"dst port\"\n");
+ return -1;
+ }
} else if (matches(*argv, "help") == 0) {
usage();
} else {
@@ -197,6 +219,14 @@ static void tunnel_key_print_key_id(FILE *f, const char *name,
fprintf(f, "\n\t%s %d", name, rta_getattr_be32(attr));
}
+static void tunnel_key_print_dst_port(FILE *f, char *name,
+ struct rtattr *attr)
+{
+ if (!attr)
+ return;
+ fprintf(f, "\n\t%s %d", name, rta_getattr_be16(attr));
+}
+
static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
{
struct rtattr *tb[TCA_TUNNEL_KEY_MAX + 1];
@@ -231,6 +261,8 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
tb[TCA_TUNNEL_KEY_ENC_IPV6_DST]);
tunnel_key_print_key_id(f, "key_id",
tb[TCA_TUNNEL_KEY_ENC_KEY_ID]);
+ tunnel_key_print_dst_port(f, "dst_port",
+ tb[TCA_TUNNEL_KEY_ENC_DST_PORT]);
break;
}
fprintf(f, " %s", action_n2a(parm->action));
--
1.8.3.1
^ permalink raw reply related
* [PATCH iproute2 0/2] Add dest UDP port to IP tunnel parameters
From: Hadar Hen Zion @ 2016-12-13 8:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Or Gerlitz, Roi Dayan, Amir Vadai, Hadar Hen Zion
Enhance IP tunnel key classification and action parameters by adding
destination UDP port.
Thanks,
Hadar
Hadar Hen Zion (2):
tc/cls_flower: Add dest UDP port to tunnel params
tc/m_tunnel_key: Add dest UDP port to tunnel key action
man/man8/tc-flower.8 | 8 +++++++-
man/man8/tc-tunnel_key.8 | 6 ++++++
tc/f_flower.c | 25 +++++++++++++++++++++++++
tc/m_tunnel_key.c | 32 ++++++++++++++++++++++++++++++++
4 files changed, 70 insertions(+), 1 deletion(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH iproute2 1/2] tc/cls_flower: Add dest UDP port to tunnel params
From: Hadar Hen Zion @ 2016-12-13 8:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Or Gerlitz, Roi Dayan, Amir Vadai, Hadar Hen Zion
In-Reply-To: <1481616467-769-1-git-send-email-hadarh@mellanox.com>
Enhance IP tunnel parameters by adding destination UDP port.
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
man/man8/tc-flower.8 | 8 +++++++-
tc/f_flower.c | 25 +++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 90fdfba..88df833 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -39,6 +39,8 @@ flower \- flow based traffic control filter
.IR KEY-ID " | {"
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
+.B enc_dst_port
+.IR UDP-PORT " | "
.SH DESCRIPTION
The
.B flower
@@ -129,11 +131,15 @@ which have to be specified in beforehand.
.BI enc_dst_ip " ADDRESS"
.TQ
.BI enc_src_ip " ADDRESS"
+.TQ
+.BI enc_dst_port " NUMBER"
Match on IP tunnel metadata. Key id
.I NUMBER
is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
.I ADDRESS
-must be a valid IPv4 or IPv6 address.
+must be a valid IPv4 or IPv6 address. Dst port
+.I NUMBER
+is a 16 bit UDP dst port.
.SH NOTES
As stated above where applicable, matches of a certain layer implicitly depend
on the matches of the next lower layer. Precisely, layer one and two matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 5dac427..653dfef 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -275,6 +275,20 @@ static int flower_parse_key_id(const char *str, int type, struct nlmsghdr *n)
return ret;
}
+static int flower_parse_enc_port(char *str, int type, struct nlmsghdr *n)
+{
+ int ret;
+ __be16 port;
+
+ ret = get_be16(&port, str, 10);
+ if (ret)
+ return -1;
+
+ addattr16(n, MAX_MSG, type, port);
+
+ return 0;
+}
+
static int flower_parse_opt(struct filter_util *qu, char *handle,
int argc, char **argv, struct nlmsghdr *n)
{
@@ -482,6 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
fprintf(stderr, "Illegal \"enc_key_id\"\n");
return -1;
}
+ } else if (matches(*argv, "enc_dst_port") == 0) {
+ NEXT_ARG();
+ ret = flower_parse_enc_port(*argv,
+ TCA_FLOWER_KEY_ENC_UDP_DST_PORT, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"enc_dst_port\"\n");
+ return -1;
+ }
} else if (matches(*argv, "action") == 0) {
NEXT_ARG();
ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -754,6 +776,9 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_key_id(f, "enc_key_id",
tb[TCA_FLOWER_KEY_ENC_KEY_ID]);
+ flower_print_port(f, "enc_dst_port",
+ tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
+
if (tb[TCA_FLOWER_FLAGS]) {
__u32 flags = rta_getattr_u32(tb[TCA_FLOWER_FLAGS]);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH V2 00/22] Broadcom RoCE Driver (bnxt_re)
From: Or Gerlitz @ 2016-12-13 6:54 UTC (permalink / raw)
To: Selvin Xavier; +Cc: dledford, linux-rdma, netdev
In-Reply-To: <1481266096-23331-1-git-send-email-selvin.xavier@broadcom.com>
On 12/9/2016 8:47 AM, Selvin Xavier wrote:
> This series introduces the RoCE driver for the Broadcom
> NetXtreme-E 10/25/40/50 gigabit RoCE HCAs.
> This driver is dependent on the bnxt_en NIC driver and is
> based on the bnxt_re branch in Doug's repository. bnxt_en changes
> required for this patch series is already available in this branch.
>
> I am preparing a git repository with these changes as per Jason's
> comment and will share the details later today.
>
> v1-> v2:
> * The license text in each file updated to reflect Dual license.
> * Makefile and Kconfig changes are pushed to the last patch
> * Moved bnxt_re_uverbs_abi.h to include/uapi/rdma folder
> * Remove duplicate structure definitions from bnxt_re_hsi.h as
> it is available in the corresponding bnxt_en header file (bnxt_hsi.h)
> * Removed some unused code reported during code review.
> * Fixed few sparse warnings
>
> Doug, Please review and consider applying this to linux-rdma repository.
Hi,
I made some quick on-the-surface static checkers etc rub on the new
driver (Doug, I used the bits in your github bnxt_re branch), there are
bunch (tons...) of smatch [1] and sparse [2]complaints along with few
checkpatch [3] things too. So... addressing them before this goes upstream?
BTW net-next's drivers/net/ethernet/broadcom/bnxt (where you put the pre
patches for the RDMA driver) is pretty much clean of that
Or.
[1] smatch
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:185 bnxt_qplib_del_sgid()
warn: impossible condition '(*sgid_tbl->hw_id + index == -1) => (0-65535
== (-1))'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:107
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:116
bnxt_qplib_rcfw_send_message() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:148
bnxt_qplib_rcfw_send_message() error: double lock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:204
bnxt_qplib_rcfw_send_message() error: double unlock 'irqsave:flags'
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:121
bnxt_re_unregister_netdev() warn: variable dereferenced before check
'rdev' (see line 118)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:140
bnxt_re_register_netdev() warn: variable dereferenced before check
'rdev' (see line 137)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:155 bnxt_re_free_msix()
warn: variable dereferenced before check 'rdev' (see line 152)
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:173 bnxt_re_request_msix()
warn: variable dereferenced before check 'rdev' (see line 171)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:172
bnxt_qplib_alloc_init_hwq() warn: unsigned '*elements' is never less
than zero.
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:387
bnxt_qplib_deinit_rcfw() warn: test_bit() takes a bit number
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:488
bnxt_qplib_alloc_sgid_tbl() warn: double check that we're allocating
correct size: 2 vs 4
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:688
bnxt_qplib_alloc_dpi_tbl() warn: consider using resource_size() here
drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c:496
bnxt_qplib_init_rcfw() warn: test_bit() takes a bit number
./include/linux/mm.h:1385 stack_guard_page_end() warn: bitwise AND
condition is false here
./include/linux/mm.h:1379 vma_growsup() warn: bitwise AND condition is
false here
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:522 show_rev() info:
ignoring unreachable code.
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:835 bnxt_re_dev_stop()
warn: was && intended here instead of ||?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1048 bnxt_re_ib_reg() warn:
curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1050 bnxt_re_ib_reg() warn:
inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_re_main.c:1190 bnxt_re_task() warn:
curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1639 __flush_sq() warn:
variable dereferenced before check 'budget' (see line 1621)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852
bnxt_qplib_cq_process_res_ud() warn: shift has higher precedence than mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1861
bnxt_qplib_cq_process_res_ud() warn: inconsistent indenting
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:2015
bnxt_qplib_cq_process_terminal() warn: variable dereferenced before
check 'budget' (see line 1997)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1607
bnxt_re_build_qp1_send_v2 warn: unused return: size = ib_ud_header_pack()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1681
bnxt_re_build_qp1_shadow_qp_recv() warn: unsigned 'sge.size' is never
less than zero.
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2066
bnxt_re_post_recv_shadow_qp warn: unused return: payload_sz =
bnxt_re_build_sgl()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2242
bnxt_re_create_cq() warn: variable dereferenced before check 'cq->umem'
(see line 2192)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2456
bnxt_re_is_loopback_packet() warn: curly braces intended?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2512
bnxt_re_process_raw_qp_pkt_rx() warn: impossible condition '(pkt_type <
0) => (0-255 < 0)'
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2578
bnxt_re_process_raw_qp_pkt_rx warn: unused return: rc =
bnxt_re_post_send_shadow_qp()
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2838 bnxt_re_dereg_mr
warn: unused return: rc = bnxt_qplib_free_fast_reg_page_list()
[2] sparse
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_main.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_re_debugfs.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_rcfw.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c
CHECK drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:111:33: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:114:30: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:277:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:278:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:279:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: expected
restricted __le32 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:280:28: got restricted
__be32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: expected
restricted __le16 [addressable] [assigned] [usertype] vlan
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:282:34: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:289:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:290:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: expected
restricted __le16 <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:291:32: got restricted
__be16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos0
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:810:18: got unsigned short
[unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: expected
restricted __le16 [addressable] [assigned] [usertype] cos1
drivers/infiniband/hw/bnxt_re/bnxt_qplib_sp.c:811:18: got unsigned short
[unsigned] [short] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:133:22: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:136:24: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: expected
restricted __le32 [addressable] [assigned] [usertype] modify_mask
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:777:25: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: warning: incorrect
type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: expected unsigned
char [unsigned] [addressable] [assigned] [usertype] path_mtu
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:822:30: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:953:26: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:956:26: warning:
restricted __le16 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast to
restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:970:26: warning: cast from
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: warning: invalid
assignment: +=
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: left side has
type int
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1211:34: right side has
type restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: expected unsigned
int [unsigned] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1333:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1343:46: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: expected unsigned
int [unsigned] [addressable] [usertype] temp32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1363:24: got restricted
__le32 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: expected
restricted __le32 [addressable] [assigned] [usertype] cq_fco_cnq_id
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1533:27: got restricted
__le16 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1796:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1849:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1852:39: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning:
restricted __le32 degrades to integer
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1904:21: warning: cast to
restricted __le64
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast to
restricted __le16
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1917:34: warning: cast
from restricted __le32
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1015:22: warning: context
imbalance in 'bnxt_qplib_lock_cqs' - wrong count at exit
drivers/infiniband/hw/bnxt_re/bnxt_qplib_fp.c:1030:28: warning: context
imbalance in 'bnxt_qplib_unlock_cqs' - unexpected unlock
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:410:72: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: expected unsigned
long long [unsigned] [long] [long long] [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:418:56: got restricted
__le64 [usertype] <noident>
drivers/infiniband/hw/bnxt_re/bnxt_qplib_res.c:729:6: warning: symbol
'bnxt_qplib_cleanup_pkey_tbl' was not declared. Should it be static?
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1725:47: got restricted
__be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: expected
unsigned int [unsigned] [usertype] imm_data_or_inv_key
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:1755:47: got restricted
__be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2627:25: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: warning:
incorrect type in assignment (different base types)
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: expected
restricted __be32 [usertype] imm_data
drivers/infiniband/hw/bnxt_re/bnxt_re_ib_verbs.c:2696:41: got unsigned
int [unsigned] [usertype] immdata_or_invrkey
[3] checkpatch
CHECK: Macro argument reuse 'req' - possible side-effects?
CHECK: Macro argument reuse 'prod' - possible side-effects?
CHECK: Macro argument reuse 'dma_addr' - possible side-effects?
CHECK: Macro argument reuse 'rdev' - possible side-effects?
CHECK: Please don't use multiple blank lines
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Alignment should match open parenthesis
CHECK: Please don't use multiple blank lines
CHECK: DEFINE_MUTEX definition without comment
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Alignment should match open parenthesis
CHECK: Please use a blank line after function/struct/union/enum declarations
^ permalink raw reply
* bloat-o-meter +32832 (was: Re: bpf: add prog_digest and expose it via fdinfo/netlink)
From: Geert Uytterhoeven @ 2016-12-13 9:29 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Linux Kernel Mailing List,
netdev@vger.kernel.org
Hi Daniel,
On Mon, Dec 12, 2016 at 6:08 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Web: https://git.kernel.org/torvalds/c/7bd509e311f408f7a5132fcdde2069af65fa05ae
> Commit: 7bd509e311f408f7a5132fcdde2069af65fa05ae
> Parent: 8d829bdb97dc3a0c9c8090b9b168ca46ea99c8d8
> Refname: refs/heads/master
> Author: Daniel Borkmann <daniel@iogearbox.net>
> AuthorDate: Sun Dec 4 23:19:41 2016 +0100
> Committer: David S. Miller <davem@davemloft.net>
> CommitDate: Mon Dec 5 15:33:11 2016 -0500
>
> bpf: add prog_digest and expose it via fdinfo/netlink
>
> When loading a BPF program via bpf(2), calculate the digest over
> the program's instruction stream and store it in struct bpf_prog's
> digest member. This is done at a point in time before any instructions
> are rewritten by the verifier. Any unstable map file descriptor
> number part of the imm field will be zeroed for the hash.
>
> fdinfo example output for progs:
>
> # cat /proc/1590/fdinfo/5
> pos: 0
> flags: 02000002
> mnt_id: 11
> prog_type: 1
> prog_jited: 1
> prog_digest: b27e8b06da22707513aa97363dfb11c7c3675d28
> memlock: 4096
>
> When programs are pinned and retrieved by an ELF loader, the loader
> can check the program's digest through fdinfo and compare it against
> one that was generated over the ELF file's program section to see
> if the program needs to be reloaded. Furthermore, this can also be
> exposed through other means such as netlink in case of a tc cls/act
> dump (or xdp in future), but also through tracepoints or other
> facilities to identify the program. Other than that, the digest can
> also serve as a base name for the work in progress kallsyms support
> of programs. The digest doesn't depend/select the crypto layer, since
> we need to keep dependencies to a minimum. iproute2 will get support
> for this facility.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -136,6 +136,71 @@ void __bpf_prog_free(struct bpf_prog *fp)
> vfree(fp);
> }
>
> +#define SHA_BPF_RAW_SIZE \
> + round_up(MAX_BPF_SIZE + sizeof(__be64) + 1, SHA_MESSAGE_BYTES)
> +
> +/* Called under verifier mutex. */
> +void bpf_prog_calc_digest(struct bpf_prog *fp)
> +{
> + const u32 bits_offset = SHA_MESSAGE_BYTES - sizeof(__be64);
> + static u32 ws[SHA_WORKSPACE_WORDS];
> + static u8 raw[SHA_BPF_RAW_SIZE];
function old new delta
raw - 32832 +32832
Congratulations!
We've found the first nominee for the v4.10-rc1 bloat-o-meter contest! :-(
Can this please be allocated dynamically? Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Synopsys Ethernet QoS
From: Niklas Cassel @ 2016-12-13 9:38 UTC (permalink / raw)
To: Giuseppe CAVALLARO, Joao Pinto, Florian Fainelli, Andy Shevchenko
Cc: David Miller, larper, rabinv, netdev, CARLOS.PALMINHA, Jie.Deng1,
Stephen Warren, pavel
In-Reply-To: <73bf8cb4-5685-2db6-529c-1de99b1fd358@st.com>
On 12/13/2016 08:22 AM, Giuseppe CAVALLARO wrote:
> On 12/12/2016 5:25 PM, Niklas Cassel wrote:
>>
>>
>> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>>> Hi,
>>>
>>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>
>>>>>> It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)
>>>>>> did
>>>>>> actually pioneer the upstreaming effort, but it is good to see people
>>>>>> from Synopsys willing to fix that in the future.
>>>>> Wait, you would like to tell that we have more than 2 drivers for the
>>>>> same (okay, same vendor) IP?!
>>>>> It's better to unify them earlier, than have n+ copies.
>>>> Unfortunately that is the case, see this email:
>>>>
>>>> https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html
>>>>
>>>> dwc_eth_qos and stmmac have some overlap. There seems to be work
>>>> underway to unify these two to begin with.
>>>>
>>>>> P.S. Though, I don't see how sxgbe got in the list. First glance on
>>>>> the code doesn't show similarities.
>>>> Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
>>>> just my cursory look at the code, it may very well be something entirely
>>>> different. The descriptor formats just look suspiciously similar.
>>>>
>>> Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
>>> instead of renaming (breaking retro-compatibility as David and Florian
>>> mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
>>> removing it. As Florian mentioned, git is capable of detecting folder restructured.
>>>
>>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>>> driver would it be possible for you to make an initial analysis of what has to
>>> be merged into Stmmac? This way the development would speed-up.
>>
>> I can answer that question.
>>
>> I've sent out 12 patches to the stmmac driver
>> (all patches are included in the current net-next tree),
>
> ok I have seen these patches applied, I had just a minor concern about
> the failure when DMA configuration is missing.
> In these years, I have noticed that, for this kind of HW, default DMA
> configuration is usually good to have a driver working. AHB, AXI
> parameters can be provided to have a best tuning or to fix know issues
> on some platforms. So IMO, we should relax the check with a warning.
> Please, consider that, the stmmac also supports very old MAC10/100
> versions where the DMA configuration was often never passed.
>
This might be a bit off-topic, but:
The patch should not affect any existing code.
All glue drivers uses stmmac_probe_config_dt,
which sets a default value if the property is missing or zero.
The PCI glue driver sets the property explicitly.
Hence, all current code should not be affected.
The error check was added as a sanity check, just in case some
new code is added, which bypasses stmmac_probe_config_dt,
lets say support for GMAC4 in the PCI glue driver.
>
>>
>> There are some performance problems with the stmmac driver though:
>>
>> When running iperf3 with 3 streams:
>> iperf3 -c 192.168.0.90 -P 3 -t 30
>> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>>
>> I get really bad fairness between the streams.
>
> Can you confirm you are using the 4.xxa version?
Yes, 4.10a.
Reading the MAC_Version register gives:
0x00004041
Where SNPSVER is bit 7:0, so 0x41.
>
> This doesn't match with Alex's experiments on ARM platforms.
>
We are also using an ARM platform.
There is no problem with throughput.
The problem is fairness between the streams,
when using for example 3 streams in iperf3.
Did Alex test this? I could not find Alex mail in the netdev archives,
could you link to it?
The problem goes away when disabling TX IRQ coalescing,
which I guess makes sense, since like David Miller said:
"the driver is using non-highres timers
to implement the TX coalescing. [...]
1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing."
We are using CONFIG_HZ=100, not CONFIG_HZ=1000.
So if there is a long time before handling interrupts,
I guess that it makes sense that one stream could
get an advantage in the net scheduler.
If I find the time, and if no one beats me to it, I will try to replace
the normal timers with HR timers + a smaller default timeout.
>
>> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
>> and we don't see the same problem.
>
> please, if you have new patch add me on CC and we will review all
> together.
I think you misunderstood me, we have a local patch for the synopsys/dwc_eth_qos.c
version, not for stmmac.
I have been using get_maintainer.pl, so you should have been added to all my patches,
but if I send any further patches, I will make sure that you are not excluded.
^ permalink raw reply
* Re: Designing a safe RX-zero-copy Memory Model for Networking
From: Mike Rapoport @ 2016-12-13 9:42 UTC (permalink / raw)
To: John Fastabend
Cc: Jesper Dangaard Brouer, netdev@vger.kernel.org, linux-mm,
Willem de Bruijn, Björn Töpel, Karlsson, Magnus,
Alexander Duyck, Mel Gorman, Tom Herbert, Brenden Blanco,
Tariq Toukan, Saeed Mahameed, Jesse Brandeburg, Kalman Meth,
Vladislav Yasevich
In-Reply-To: <584EB8DF.8000308@gmail.com>
On Mon, Dec 12, 2016 at 06:49:03AM -0800, John Fastabend wrote:
> On 16-12-12 06:14 AM, Mike Rapoport wrote:
> >>
> > We were not considered using XDP yet, so we've decided to limit the initial
> > implementation to macvtap because we can ensure correspondence between a
> > NIC queue and virtual NIC, which is not the case with more generic tap
> > device. It could be that use of XDP will allow for a generic solution for
> > virtio case as well.
>
> Interesting this was one of the original ideas behind the macvlan
> offload mode. iirc Vlad also was interested in this.
>
> I'm guessing this was used because of the ability to push macvlan onto
> its own queue?
Yes, with a queue dedicated to a virtual NIC we only need to ensure that
guest memory is used for RX buffers.
> >>
> >>> Have you considered using "push" model for setting the NIC's RX memory?
> >>
> >> I don't understand what you mean by a "push" model?
> >
> > Currently, memory allocation in NIC drivers boils down to alloc_page with
> > some wrapping code. I see two possible ways to make NIC use of some
> > preallocated pages: either NIC driver will call an API (probably different
> > from alloc_page) to obtain that memory, or there will be NDO API that
> > allows to set the NIC's RX buffers. I named the later case "push".
>
> I prefer the ndo op. This matches up well with AF_PACKET model where we
> have "slots" and offload is just a transparent "push" of these "slots"
> to the driver. Below we have a snippet of our proposed API,
>
> (https://patchwork.ozlabs.org/patch/396714/ note the descriptor mapping
> bits will be dropped)
>
> + * int (*ndo_direct_qpair_page_map) (struct vm_area_struct *vma,
> + * struct net_device *dev)
> + * Called to map queue pair range from split_queue_pairs into
> + * mmap region.
> +
>
> > +
> > +static int
> > +ixgbe_ndo_qpair_page_map(struct vm_area_struct *vma, struct net_device *dev)
> > +{
> > + struct ixgbe_adapter *adapter = netdev_priv(dev);
> > + phys_addr_t phy_addr = pci_resource_start(adapter->pdev, 0);
> > + unsigned long pfn_rx = (phy_addr + RX_DESC_ADDR_OFFSET) >> PAGE_SHIFT;
> > + unsigned long pfn_tx = (phy_addr + TX_DESC_ADDR_OFFSET) >> PAGE_SHIFT;
> > + unsigned long dummy_page_phy;
> > + pgprot_t pre_vm_page_prot;
> > + unsigned long start;
> > + unsigned int i;
> > + int err;
> > +
> > + if (!dummy_page_buf) {
> > + dummy_page_buf = kzalloc(PAGE_SIZE_4K, GFP_KERNEL);
> > + if (!dummy_page_buf)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < PAGE_SIZE_4K / sizeof(unsigned int); i++)
> > + dummy_page_buf[i] = 0xdeadbeef;
> > + }
> > +
> > + dummy_page_phy = virt_to_phys(dummy_page_buf);
> > + pre_vm_page_prot = vma->vm_page_prot;
> > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> > +
> > + /* assume the vm_start is 4K aligned address */
> > + for (start = vma->vm_start;
> > + start < vma->vm_end;
> > + start += PAGE_SIZE_4K) {
> > + if (start == vma->vm_start + RX_DESC_ADDR_OFFSET) {
> > + err = remap_pfn_range(vma, start, pfn_rx, PAGE_SIZE_4K,
> > + vma->vm_page_prot);
> > + if (err)
> > + return -EAGAIN;
> > + } else if (start == vma->vm_start + TX_DESC_ADDR_OFFSET) {
> > + err = remap_pfn_range(vma, start, pfn_tx, PAGE_SIZE_4K,
> > + vma->vm_page_prot);
> > + if (err)
> > + return -EAGAIN;
> > + } else {
> > + unsigned long addr = dummy_page_phy > PAGE_SHIFT;
> > +
> > + err = remap_pfn_range(vma, start, addr, PAGE_SIZE_4K,
> > + pre_vm_page_prot);
> > + if (err)
> > + return -EAGAIN;
> > + }
> > + }
> > + return 0;
> > +}
> > +
>
> Any thoughts on something like the above? We could push it when net-next
> opens. One piece that fits naturally into vhost/macvtap is the kicks and
> queue splicing are already there so no need to implement this making the
> above patch much simpler.
Sorry, but I don't quite follow you here. The vhost does not use vma
mappings, it just sees a bunch of pages pointed by the vring descriptors...
> .John
--
Sincerely yours,
Mike.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v4 0/4] vsock: cancel connect packets when failing to connect
From: Stefan Hajnoczi @ 2016-12-13 9:50 UTC (permalink / raw)
To: Peng Tao
Cc: kvm, netdev, virtualization, Stefan Hajnoczi, David Miller,
Jorgen Hansen
In-Reply-To: <1481545269-18104-1-git-send-email-bergwolf@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1792 bytes --]
On Mon, Dec 12, 2016 at 08:21:05PM +0800, Peng Tao wrote:
> Currently, if a connect call fails on a signal or timeout (e.g., guest is still
> in the process of starting up), we'll just return to caller and leave the connect
> packet queued and they are sent even though the connection is considered a failure,
> which can confuse applications with unwanted false connect attempt.
>
> The patchset enables vsock (both host and guest) to cancel queued packets when
> a connect attempt is considered to fail.
>
> v4 changelog:
> - drop two unnecessary void * cast
> - update new callback commnet
> v3 changelog:
> - define cancel_pkt callback in struct vsock_transport rather than struct virtio_transport
> - rename virtio_vsock_pkt->vsk to virtio_vsock_pkt->cancel_token
> v2 changelog:
> - fix queued_replies counting and resume tx/rx when necessary
>
> Cheers,
> Tao
>
> Peng Tao (4):
> vsock: track pkt owner vsock
> vhost-vsock: add pkt cancel capability
> vsock: add pkt cancel capability
> vsock: cancel packets when failing to connect
>
> drivers/vhost/vsock.c | 41 ++++++++++++++++++++++++++++++++
> include/linux/virtio_vsock.h | 2 ++
> include/net/af_vsock.h | 3 +++
> net/vmw_vsock/af_vsock.c | 14 +++++++++++
> net/vmw_vsock/virtio_transport.c | 42 +++++++++++++++++++++++++++++++++
> net/vmw_vsock/virtio_transport_common.c | 7 ++++++
> 6 files changed, 109 insertions(+)
>
> --
> 2.7.4
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: Synopsys Ethernet QoS
From: Giuseppe CAVALLARO @ 2016-12-13 9:51 UTC (permalink / raw)
To: Niklas Cassel, Joao Pinto, Florian Fainelli, Andy Shevchenko
Cc: David Miller, larper, rabinv, netdev, CARLOS.PALMINHA, Jie.Deng1,
Stephen Warren, pavel
In-Reply-To: <99424968-ad8f-fec6-ebcf-ab7b19ee5486@axis.com>
Hello Niklas
On 12/13/2016 10:38 AM, Niklas Cassel wrote:
> On 12/13/2016 08:22 AM, Giuseppe CAVALLARO wrote:
>> On 12/12/2016 5:25 PM, Niklas Cassel wrote:
>>>
>>>
>>> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>>>> Hi,
>>>>
>>>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>>
>>>>>>> It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)
>>>>>>> did
>>>>>>> actually pioneer the upstreaming effort, but it is good to see people
>>>>>>> from Synopsys willing to fix that in the future.
>>>>>> Wait, you would like to tell that we have more than 2 drivers for the
>>>>>> same (okay, same vendor) IP?!
>>>>>> It's better to unify them earlier, than have n+ copies.
>>>>> Unfortunately that is the case, see this email:
>>>>>
>>>>> https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html
>>>>>
>>>>> dwc_eth_qos and stmmac have some overlap. There seems to be work
>>>>> underway to unify these two to begin with.
>>>>>
>>>>>> P.S. Though, I don't see how sxgbe got in the list. First glance on
>>>>>> the code doesn't show similarities.
>>>>> Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
>>>>> just my cursory look at the code, it may very well be something entirely
>>>>> different. The descriptor formats just look suspiciously similar.
>>>>>
>>>> Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
>>>> instead of renaming (breaking retro-compatibility as David and Florian
>>>> mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
>>>> removing it. As Florian mentioned, git is capable of detecting folder restructured.
>>>>
>>>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>>>> driver would it be possible for you to make an initial analysis of what has to
>>>> be merged into Stmmac? This way the development would speed-up.
>>>
>>> I can answer that question.
>>>
>>> I've sent out 12 patches to the stmmac driver
>>> (all patches are included in the current net-next tree),
>>
>> ok I have seen these patches applied, I had just a minor concern about
>> the failure when DMA configuration is missing.
>> In these years, I have noticed that, for this kind of HW, default DMA
>> configuration is usually good to have a driver working. AHB, AXI
>> parameters can be provided to have a best tuning or to fix know issues
>> on some platforms. So IMO, we should relax the check with a warning.
>> Please, consider that, the stmmac also supports very old MAC10/100
>> versions where the DMA configuration was often never passed.
>>
>
> This might be a bit off-topic, but:
yes indeed ;-)
> The patch should not affect any existing code.
> All glue drivers uses stmmac_probe_config_dt,
> which sets a default value if the property is missing or zero.
> The PCI glue driver sets the property explicitly.
> Hence, all current code should not be affected.
> The error check was added as a sanity check, just in case some
> new code is added, which bypasses stmmac_probe_config_dt,
> lets say support for GMAC4 in the PCI glue driver.
>
ok
>>
>>>
>>> There are some performance problems with the stmmac driver though:
>>>
>>> When running iperf3 with 3 streams:
>>> iperf3 -c 192.168.0.90 -P 3 -t 30
>>> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>>>
>>> I get really bad fairness between the streams.
>>
>> Can you confirm you are using the 4.xxa version?
>
> Yes, 4.10a.
> Reading the MAC_Version register gives:
> 0x00004041
> Where SNPSVER is bit 7:0, so 0x41.
ok
>
>
>>
>> This doesn't match with Alex's experiments on ARM platforms.
>>
>
> We are also using an ARM platform.
> There is no problem with throughput.
ok
> The problem is fairness between the streams,
> when using for example 3 streams in iperf3.
hmm I let Alex to reply.
AFAIK, other people played with stream tests and
no issue but, if you get problems so we have to
investigate.
> Did Alex test this? I could not find Alex mail in the netdev archives,
> could you link to it?
> The problem goes away when disabling TX IRQ coalescing,
> which I guess makes sense, since like David Miller said:
>
> "the driver is using non-highres timers
> to implement the TX coalescing. [...]
> 1 HZ, which is the lowest granularity of non-highres timers in the
> kernel, is variable as well as already too large of a delay for
> effective TX coalescing."
>
> We are using CONFIG_HZ=100, not CONFIG_HZ=1000.
1000 was a default on our platforms
>
> So if there is a long time before handling interrupts,
> I guess that it makes sense that one stream could
> get an advantage in the net scheduler.
ok
>
> If I find the time, and if no one beats me to it, I will try to replace
> the normal timers with HR timers + a smaller default timeout.
that's great.
>>
>>> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
>>> and we don't see the same problem.
>>
>> please, if you have new patch add me on CC and we will review all
>> together.
>
> I think you misunderstood me, we have a local patch for the synopsys/dwc_eth_qos.c
> version, not for stmmac.
oops, sorry
>
> I have been using get_maintainer.pl, so you should have been added to all my patches,
> but if I send any further patches, I will make sure that you are not excluded.
thanks a lot
Regards
Peppe
>
>
^ permalink raw reply
* Re: [PATCH v4 2/4] vhost-vsock: add pkt cancel capability
From: Jorgen S. Hansen @ 2016-12-13 10:04 UTC (permalink / raw)
To: Peng Tao
Cc: netdev@vger.kernel.org, Stefan Hajnoczi, David Miller,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org
In-Reply-To: <1481545269-18104-3-git-send-email-bergwolf@gmail.com>
> On Dec 12, 2016, at 1:21 PM, Peng Tao <bergwolf@gmail.com> wrote:
>
> To allow canceling all packets of a connection.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
> drivers/vhost/vsock.c | 41 +++++++++++++++++++++++++++++++++++++++++
> include/net/af_vsock.h | 3 +++
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index a504e2e0..fef8808 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -218,6 +218,46 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
> return len;
> }
>
> +static int
> +vhost_transport_cancel_pkt(struct vsock_sock *vsk)
> +{
> + struct vhost_vsock *vsock;
> + struct virtio_vsock_pkt *pkt, *n;
> + int cnt = 0;
> + LIST_HEAD(freeme);
> +
> + /* Find the vhost_vsock according to guest context id */
> + vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
> + if (!vsock)
> + return -ENODEV;
> +
> + spin_lock_bh(&vsock->send_pkt_list_lock);
> + list_for_each_entry_safe(pkt, n, &vsock->send_pkt_list, list) {
> + if (pkt->cancel_token != vsk)
> + continue;
> + list_move(&pkt->list, &freeme);
> + }
> + spin_unlock_bh(&vsock->send_pkt_list_lock);
> +
> + list_for_each_entry_safe(pkt, n, &freeme, list) {
> + if (pkt->reply)
> + cnt++;
> + list_del(&pkt->list);
> + virtio_transport_free_pkt(pkt);
> + }
> +
> + if (cnt) {
> + struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
> + int new_cnt;
> +
> + new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
> + if (new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
> + vhost_poll_queue(&tx_vq->poll);
> + }
> +
> + return 0;
> +}
> +
> static struct virtio_vsock_pkt *
> vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
> unsigned int out, unsigned int in)
> @@ -664,6 +704,7 @@ static struct virtio_transport vhost_transport = {
> .release = virtio_transport_release,
> .connect = virtio_transport_connect,
> .shutdown = virtio_transport_shutdown,
> + .cancel_pkt = vhost_transport_cancel_pkt,
>
> .dgram_enqueue = virtio_transport_dgram_enqueue,
> .dgram_dequeue = virtio_transport_dgram_dequeue,
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index f275896..f32ed9a 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -100,6 +100,9 @@ struct vsock_transport {
> void (*destruct)(struct vsock_sock *);
> void (*release)(struct vsock_sock *);
>
> + /* Cancel all pending packets sent on vsock. */
> + int (*cancel_pkt)(struct vsock_sock *vsk);
> +
> /* Connections. */
> int (*connect)(struct vsock_sock *);
>
> --
> 2.7.4
>
Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
^ permalink raw reply
* Re: bloat-o-meter +32832 (was: Re: bpf: add prog_digest and expose it via fdinfo/netlink)
From: Daniel Borkmann @ 2016-12-13 10:06 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Alexei Starovoitov, Linux Kernel Mailing List,
netdev@vger.kernel.org
In-Reply-To: <CAMuHMdUoadaRw9dAOkpzn9MKV=3AQhmtfMu_zzoT6_VmFaePQg@mail.gmail.com>
On 12/13/2016 10:29 AM, Geert Uytterhoeven wrote:
> Hi Daniel,
>
> On Mon, Dec 12, 2016 at 6:08 PM, Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org> wrote:
>> Web: https://git.kernel.org/torvalds/c/7bd509e311f408f7a5132fcdde2069af65fa05ae
>> Commit: 7bd509e311f408f7a5132fcdde2069af65fa05ae
>> Parent: 8d829bdb97dc3a0c9c8090b9b168ca46ea99c8d8
>> Refname: refs/heads/master
>> Author: Daniel Borkmann <daniel@iogearbox.net>
>> AuthorDate: Sun Dec 4 23:19:41 2016 +0100
>> Committer: David S. Miller <davem@davemloft.net>
>> CommitDate: Mon Dec 5 15:33:11 2016 -0500
>>
>> bpf: add prog_digest and expose it via fdinfo/netlink
>>
>> When loading a BPF program via bpf(2), calculate the digest over
>> the program's instruction stream and store it in struct bpf_prog's
>> digest member. This is done at a point in time before any instructions
>> are rewritten by the verifier. Any unstable map file descriptor
>> number part of the imm field will be zeroed for the hash.
>>
>> fdinfo example output for progs:
>>
>> # cat /proc/1590/fdinfo/5
>> pos: 0
>> flags: 02000002
>> mnt_id: 11
>> prog_type: 1
>> prog_jited: 1
>> prog_digest: b27e8b06da22707513aa97363dfb11c7c3675d28
>> memlock: 4096
>>
>> When programs are pinned and retrieved by an ELF loader, the loader
>> can check the program's digest through fdinfo and compare it against
>> one that was generated over the ELF file's program section to see
>> if the program needs to be reloaded. Furthermore, this can also be
>> exposed through other means such as netlink in case of a tc cls/act
>> dump (or xdp in future), but also through tracepoints or other
>> facilities to identify the program. Other than that, the digest can
>> also serve as a base name for the work in progress kallsyms support
>> of programs. The digest doesn't depend/select the crypto layer, since
>> we need to keep dependencies to a minimum. iproute2 will get support
>> for this facility.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Acked-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -136,6 +136,71 @@ void __bpf_prog_free(struct bpf_prog *fp)
>> vfree(fp);
>> }
>>
>> +#define SHA_BPF_RAW_SIZE \
>> + round_up(MAX_BPF_SIZE + sizeof(__be64) + 1, SHA_MESSAGE_BYTES)
>> +
>> +/* Called under verifier mutex. */
>> +void bpf_prog_calc_digest(struct bpf_prog *fp)
>> +{
>> + const u32 bits_offset = SHA_MESSAGE_BYTES - sizeof(__be64);
>> + static u32 ws[SHA_WORKSPACE_WORDS];
>> + static u8 raw[SHA_BPF_RAW_SIZE];
>
> function old new delta
> raw - 32832 +32832
>
> Congratulations!
> We've found the first nominee for the v4.10-rc1 bloat-o-meter contest! :-(
>
> Can this please be allocated dynamically? Thanks!
Sure, should be okay since mostly slow-path. Will send a patch.
^ permalink raw reply
* Re: [PATCH net-next 17/27] OVS: remove assumptions about VLAN_TAG_PRESENT bit
From: Jiri Benc @ 2016-12-13 10:40 UTC (permalink / raw)
To: Michał Mirosław
Cc: open list:OPENVSWITCH, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e44219bc56d3e44aa0711c83c626adabf4c4ecd8.1481586602.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
On Tue, 13 Dec 2016 01:12:38 +0100 (CET), Michał Mirosław wrote:
> @@ -850,20 +848,11 @@ static int validate_vlan_from_nlattrs(const struct sw_flow_match *match,
> return -EINVAL;
> }
>
> - if (a[OVS_KEY_ATTR_VLAN])
> - tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
> -
> - if (!(tci & htons(VLAN_TAG_PRESENT))) {
> - if (tci) {
> - OVS_NLERR(log, "%s TCI does not have VLAN_TAG_PRESENT bit set.",
> - (inner) ? "C-VLAN" : "VLAN");
> - return -EINVAL;
> - } else if (nla_len(a[OVS_KEY_ATTR_ENCAP])) {
> - /* Corner case for truncated VLAN header. */
> - OVS_NLERR(log, "Truncated %s header has non-zero encap attribute.",
> - (inner) ? "C-VLAN" : "VLAN");
> - return -EINVAL;
> - }
> + if (!a[OVS_KEY_ATTR_VLAN] && nla_len(a[OVS_KEY_ATTR_ENCAP])) {
> + /* Corner case for truncated VLAN header. */
> + OVS_NLERR(log, "Truncated %s header has non-zero encap attribute.",
> + (inner) ? "C-VLAN" : "VLAN");
> + return -EINVAL;
This looks wrong. The zero value of nla_get_be16(a[OVS_KEY_ATTR_VLAN])
together with empty a[OVS_KEY_ATTR_ENCAP] means truncated VLAN header
and this needs to be preserved.
In other words, you need to check also !nla_get_be16(a[OVS_KEY_ATTR_VLAN])
here.
Overall, this whole patch looks very suspicious from the very
beginning. The xors used are strong indication that something is not
right.
And indeed, you're breaking uAPI compatibility. Previously, the
VLAG_TAG_PRESENT bit set in OVS_KEY_ATTR_VLAN caused the flow to match
on packets with VLAN tag present. After this patch, it causes the flow
to match only on those packets that have a certain value of
VLAN_CFI_MASK in their VLAN tag (I haven't bother deciphering what
value is checked after all these xors, it's as well possible that it
will also match on packets _without_ any VLAN header).
You have to preserve the old meaning of VLAN_TAG_PRESENT in
OVS_KEY_ATTR_VLAN. When doing flow lookups, VLAN_TAG_PRESENT must match
on and only on packets that have VLAN tag present, irrespective of their
VLAN_CFI_MASK.
If you want to introduce support for lookups on VLAN_CFI_MASK, you'll
have to do that by introducing a new netlink attribute.
Jiri
_______________________________________________
dev mailing list
dev@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
^ permalink raw reply
* Re: netlink: GPF in sock_sndtimeo
From: Richard Guy Briggs @ 2016-12-13 10:52 UTC (permalink / raw)
To: Cong Wang
Cc: linux-audit, Paul Moore, Dmitry Vyukov, David Miller,
Johannes Berg, Florian Westphal, Eric Dumazet, Herbert Xu, netdev,
LKML, syzkaller
In-Reply-To: <CAM_iQpUejChnsMWx3Z59GTuxZMeBjgrs2rhd7gsQAKoxE8YuDg@mail.gmail.com>
On 2016-12-12 16:10, Cong Wang wrote:
> On Mon, Dec 12, 2016 at 2:02 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2016-12-09 20:13, Cong Wang wrote:
> >> Netlink notifier can safely be converted to blocking one, I will send
> >> a patch.
> >
> > I had a quick look at how that might happen. The netlink notifier chain
> > is atomic. Would the registered callback funciton need to spawn a
> > one-time thread to avoid blocking?
>
> It is already non-atomic now:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=efa172f42836477bf1ac3c9a3053140df764699c
Ok, that is recent... It is still less attractive as you point out due
to the overhead, but still worth considering if we can't find another
way.
> > I had a look at your patch. It looks attractively simple. The audit
> > next tree has patches queued that add an audit_reset function that will
> > require more work. I still see some potential gaps.
> >
> > - If the process messes up (or the sock lookup messes up) it is reset
> > in the kauditd thread under the audit_cmd_mutex.
> >
> > - If the process exits normally or is replaced due to an audit_replace
> > error, it is reset from audit_receive_skb under the audit_cmd_mutex.
> >
> > - If the process dies before the kauditd thread notices, either reap it
> > via notifier callback or it needs a check on net exit to reset. This
> > last one appears necessary to decrement the sock refcount so the sock
> > can be released in netlink_kernel_release().
> >
> > If we want to be proactive and use the netlink notifier, we assume the
> > overhead of adding to the netlink notifier chain and eliminate all the
> > other reset calls under the kauditd thread. If we are ok being
> > reactionary, then we'll at least need the net exit check on audit_sock.
>
> I don't see why we need to check it in net exit if we use refcnt,
> because we have two different users of audit_sock: kauditd and
> netns, if both take care of refcnt properly, we don't need to worry
> about who is the last, no matter what failures occur in what order.
It is actually the audit_pid and audit_nlk_portid that I care about
more. The audit daemon could vanish or close the socket while the
kernel sock to which it was attached is still quite valid. Accessing
the set of three atomically is the urge. I wonder if it makes more
sense to test for the presence of auditd using audit_sock rather than
audit_pid, but still keep audit_pid for our reporting and replacement
strategy. Another idea would be to put the three in one struct.
Can someone explain how they think the original test was able to trigger
this GPF? Network namespace shutdown while something pretended to set
up a new auditd? That's impressive for a fuzzer if that's the case...
Is there an strace? I guess it is all in test().
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCHv2 4/5] sh_eth: enable wake-on-lan for sh7743
From: Geert Uytterhoeven @ 2016-12-13 10:59 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Sergei Shtylyov, Simon Horman, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161212160931.6478-5-niklas.soderlund+renesas@ragnatech.se>
On Mon, Dec 12, 2016 at 5:09 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> This is based on public datasheet for sh7743 which shows it have the
sh7734 (also in the one-line summary)
it has
> same behavior and registers for WoL as other versions of sh_eth.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Synopsys Ethernet QoS
From: Joao Pinto @ 2016-12-13 11:49 UTC (permalink / raw)
To: Niklas Cassel, Joao Pinto, Florian Fainelli, Andy Shevchenko
Cc: David Miller, Giuseppe CAVALLARO, larper, rabinv, netdev,
CARLOS.PALMINHA, Jie.Deng1, Stephen Warren, pavel
In-Reply-To: <1d445ec1-deb8-6e36-39c4-6813c446095f@axis.com>
Hi Niklas,
Às 4:25 PM de 12/12/2016, Niklas Cassel escreveu:
>
>
> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>> Hi,
>>
>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
(snip...)
>>
>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>> driver would it be possible for you to make an initial analysis of what has to
>> be merged into Stmmac? This way the development would speed-up.
>
> I can answer that question.
>
> I've sent out 12 patches to the stmmac driver
> (all patches are included in the current net-next tree),
> with these patches the stmmac driver works properly on Axis hardware
> (we use Synopsys GMAC 4.10a synthesized with multiple TX queues).
> stmmac's DT binding has also been extended with properties that
> existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.
>
> Since we have no problem updating the DTB together with the kernel,
> we will simply move to using the start using the stmmac driver,
> with stmmac's DT binding.
>
> However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
> I don't how easy it would be for them to switch to stmmac's DT binding.
> (Adding Stephen Warren to CC.)
>
> The reset sequence that Lars Persson was worried about is not an issue
> with the stmmac driver.
Great! So you saying that stmmac works great with AXIS hardware and no need to
merge specific code from AXIS' *qos* driver?
>
>
>
>
> There are some performance problems with the stmmac driver though:
>
> When running iperf3 with 3 streams:
> iperf3 -c 192.168.0.90 -P 3 -t 30
> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>
> I get really bad fairness between the streams.
>
> This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
> Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.
> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
> and we don't see the same problem.
>
> Also netperf TCP_RR and UDP_RR gives really bad results compared to the
> dwceqos driver (without IRQ coalescing).
> 2000 transactions/sec vs 9000 transactions/sec.
> Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
> gives the same performance. I guess it's a trade off, low CPU usage
> vs low latency, so I don't know how important TCP_RR/UDP_RR really is.
>
> The best thing would be to get a good working TX IRQ coalesce
> implementation with HR timers in stmmac.
> Perhaps it should also be investigated if the RX interrupt watchdog
> timeout should have a lower default value.
>
>
>
>>
>> Thanks to all.
>>
>> Joao
>
^ permalink raw reply
* Re: Synopsys Ethernet QoS
From: Niklas Cassel @ 2016-12-13 12:31 UTC (permalink / raw)
To: Joao Pinto, Florian Fainelli, Andy Shevchenko
Cc: David Miller, Giuseppe CAVALLARO, larper, rabinv, netdev,
CARLOS.PALMINHA, Jie.Deng1, Stephen Warren, pavel
In-Reply-To: <9bedb947-e27b-6bd8-ec11-6389afd189ec@synopsys.com>
On 12/13/2016 12:49 PM, Joao Pinto wrote:
> Hi Niklas,
>
> Às 4:25 PM de 12/12/2016, Niklas Cassel escreveu:
>>
>> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>>> Hi,
>>>
>>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> (snip...)
>
>
>>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>>> driver would it be possible for you to make an initial analysis of what has to
>>> be merged into Stmmac? This way the development would speed-up.
>> I can answer that question.
>>
>> I've sent out 12 patches to the stmmac driver
>> (all patches are included in the current net-next tree),
>> with these patches the stmmac driver works properly on Axis hardware
>> (we use Synopsys GMAC 4.10a synthesized with multiple TX queues).
>> stmmac's DT binding has also been extended with properties that
>> existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.
>>
>> Since we have no problem updating the DTB together with the kernel,
>> we will simply move to using the start using the stmmac driver,
>> with stmmac's DT binding.
>>
>> However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
>> I don't how easy it would be for them to switch to stmmac's DT binding.
>> (Adding Stephen Warren to CC.)
>>
>> The reset sequence that Lars Persson was worried about is not an issue
>> with the stmmac driver.
> Great! So you saying that stmmac works great with AXIS hardware and no need to
> merge specific code from AXIS' *qos* driver?
Yes. From Axis point of view, we are done.
stmmac works and we will move to that driver + DT binding.
However, it seems like Stephen Warren will NAK if you try to remove
synopsys/dwc_eth_qos.c before
Documentation/devicetree/bindings/net/stmmac.txt
is compatible with
Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
You might want to sync with him. I have no idea, but perhaps they are
only using a subset of all the available properties. Perhaps,
only implementing what they are using might be enough, I don't know.
I couldn't find their DTS in arch/arm/dts.
I guess you might want to know David Miller's opinion,
since he's the one who decides in the end.
>>
>>
>>
>> There are some performance problems with the stmmac driver though:
>>
>> When running iperf3 with 3 streams:
>> iperf3 -c 192.168.0.90 -P 3 -t 30
>> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>>
>> I get really bad fairness between the streams.
>>
>> This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
>> Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.
>> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
>> and we don't see the same problem.
>>
>> Also netperf TCP_RR and UDP_RR gives really bad results compared to the
>> dwceqos driver (without IRQ coalescing).
>> 2000 transactions/sec vs 9000 transactions/sec.
>> Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
>> gives the same performance. I guess it's a trade off, low CPU usage
>> vs low latency, so I don't know how important TCP_RR/UDP_RR really is.
>>
>> The best thing would be to get a good working TX IRQ coalesce
>> implementation with HR timers in stmmac.
>> Perhaps it should also be investigated if the RX interrupt watchdog
>> timeout should have a lower default value.
>>
>>
>>
>>> Thanks to all.
>>>
>>> Joao
^ permalink raw reply
* [PATCH iproute2 V2 1/2] tc: flower: Fix typo and style in flower man page
From: Roi Dayan @ 2016-12-13 12:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Amir Vadai, Hadar Hen Zion, Roi Dayan
In-Reply-To: <1481632742-18020-1-git-send-email-roid@mellanox.com>
Replace vlan_eth_type with vlan_ethtype.
Fixes: 745d91726006 ("tc: flower: Introduce vlan support")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Hadar Hen Zion <hadarh@mellanox.com>
---
man/man8/tc-flower.8 | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 90fdfba..15bc32f 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -27,7 +27,7 @@ flower \- flow based traffic control filter
.IR VID " | "
.B vlan_prio
.IR PRIORITY " | "
-.BR vlan_eth_type " { " ipv4 " | " ipv6 " | "
+.BR vlan_ethtype " { " ipv4 " | " ipv6 " | "
.IR ETH_TYPE " } | "
.BR ip_proto " { " tcp " | " udp " | " sctp " | " icmp " | " icmpv6 " | "
.IR IP_PROTO " } | { "
@@ -82,16 +82,16 @@ Match on vlan tag id.
.I VID
is an unsigned 12bit value in decimal format.
.TP
-.BI vlan_prio " priority"
+.BI vlan_prio " PRIORITY"
Match on vlan tag priority.
.I PRIORITY
is an unsigned 3bit value in decimal format.
.TP
-.BI vlan_eth_type " VLAN_ETH_TYPE"
+.BI vlan_ethtype " VLAN_ETH_TYPE"
Match on layer three protocol.
-.I ETH_TYPE
+.I VLAN_ETH_TYPE
may be either
-.BR ipv4 , ipv6
+.BR ipv4 ", " ipv6
or an unsigned 16bit value in hexadecimal format.
.TP
.BI ip_proto " IP_PROTO"
--
2.7.4
^ permalink raw reply related
* [PATCH iproute2 V2 0/2] Man pages fixes
From: Roi Dayan @ 2016-12-13 12:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Amir Vadai, Hadar Hen Zion, Roi Dayan
Hi,
The 2 patches are man page related only.
First fixes a typo and second adding missing man page to the Makefile.
Thanks,
Roi
v2:
- style fix in flower man page
Roi Dayan (2):
tc: flower: Fix typo and style in flower man page
tc: tunnel_key: Add tc-tunnel_key man page to Makefile
man/man8/Makefile | 1 +
man/man8/tc-flower.8 | 10 +++++-----
2 files changed, 6 insertions(+), 5 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH iproute2 V2 2/2] tc: tunnel_key: Add tc-tunnel_key man page to Makefile
From: Roi Dayan @ 2016-12-13 12:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Amir Vadai, Hadar Hen Zion, Roi Dayan
In-Reply-To: <1481632742-18020-1-git-send-email-roid@mellanox.com>
To be installed with the other man pages.
Fixes: d57639a475a9 ("tc/act_tunnel: Introduce ip tunnel action")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Amir Vadai <amir@vadai.me>
---
man/man8/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/man/man8/Makefile b/man/man8/Makefile
index de6f249..d4cb01a 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -17,6 +17,7 @@ MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 rtpr.8 ss.
tc-tcindex.8 tc-u32.8 tc-matchall.8 \
tc-connmark.8 tc-csum.8 tc-mirred.8 tc-nat.8 tc-pedit.8 tc-police.8 \
tc-simple.8 tc-skbedit.8 tc-vlan.8 tc-xt.8 tc-ife.8 tc-skbmod.8 \
+ tc-tunnel_key.8 \
devlink.8 devlink-dev.8 devlink-monitor.8 devlink-port.8 devlink-sb.8
all: $(TARGETS)
--
2.7.4
^ permalink raw reply related
* Re: Synopsys Ethernet QoS
From: Lars Persson @ 2016-12-13 12:50 UTC (permalink / raw)
To: Niklas Cassel
Cc: Joao Pinto, Florian Fainelli, Andy Shevchenko, David Miller,
Giuseppe CAVALLARO, Rabin Vincent, netdev,
CARLOS.PALMINHA@synopsys.com, Jie.Deng1@synopsys.com,
Stephen Warren, pavel@ucw.cz
In-Reply-To: <b6cbe6e6-c998-302a-36d3-2183acb076d9@axis.com>
> 13 dec. 2016 kl. 13:31 skrev Niklas Cassel <Niklas.Cassel@axis.com>:
>
>
>
>> On 12/13/2016 12:49 PM, Joao Pinto wrote:
>> Hi Niklas,
>>
>> Às 4:25 PM de 12/12/2016, Niklas Cassel escreveu:
>>>
>>>> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>>>> Hi,
>>>>
>>>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>>>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> (snip...)
>>
>>
>>>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>>>> driver would it be possible for you to make an initial analysis of what has to
>>>> be merged into Stmmac? This way the development would speed-up.
>>> I can answer that question.
>>>
>>> I've sent out 12 patches to the stmmac driver
>>> (all patches are included in the current net-next tree),
>>> with these patches the stmmac driver works properly on Axis hardware
>>> (we use Synopsys GMAC 4.10a synthesized with multiple TX queues).
>>> stmmac's DT binding has also been extended with properties that
>>> existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.
>>>
>>> Since we have no problem updating the DTB together with the kernel,
>>> we will simply move to using the start using the stmmac driver,
>>> with stmmac's DT binding.
>>>
>>> However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
>>> I don't how easy it would be for them to switch to stmmac's DT binding.
>>> (Adding Stephen Warren to CC.)
>>>
>>> The reset sequence that Lars Persson was worried about is not an issue
>>> with the stmmac driver.
>> Great! So you saying that stmmac works great with AXIS hardware and no need to
>> merge specific code from AXIS' *qos* driver?
>
> Yes. From Axis point of view, we are done.
> stmmac works and we will move to that driver + DT binding.
>
> However, it seems like Stephen Warren will NAK if you try to remove
> synopsys/dwc_eth_qos.c before
> Documentation/devicetree/bindings/net/stmmac.txt
> is compatible with
> Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
>
> You might want to sync with him. I have no idea, but perhaps they are
> only using a subset of all the available properties. Perhaps,
> only implementing what they are using might be enough, I don't know.
> I couldn't find their DTS in arch/arm/dts.
> I guess you might want to know David Miller's opinion,
> since he's the one who decides in the end.
I will also NACK removal of dwc_eth_qos.c until the binding is implemented elsewhere.
>
>>>
>>>
>>>
>>> There are some performance problems with the stmmac driver though:
>>>
>>> When running iperf3 with 3 streams:
>>> iperf3 -c 192.168.0.90 -P 3 -t 30
>>> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>>>
>>> I get really bad fairness between the streams.
>>>
>>> This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
>>> Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.
>>> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
>>> and we don't see the same problem.
>>>
>>> Also netperf TCP_RR and UDP_RR gives really bad results compared to the
>>> dwceqos driver (without IRQ coalescing).
>>> 2000 transactions/sec vs 9000 transactions/sec.
>>> Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
>>> gives the same performance. I guess it's a trade off, low CPU usage
>>> vs low latency, so I don't know how important TCP_RR/UDP_RR really is.
>>>
>>> The best thing would be to get a good working TX IRQ coalesce
>>> implementation with HR timers in stmmac.
>>> Perhaps it should also be investigated if the RX interrupt watchdog
>>> timeout should have a lower default value.
>>>
>>>
>>>
>>>> Thanks to all.
>>>>
>>>> Joao
>
^ permalink raw reply
* Re: [PATCHv2 3/5] sh_eth: enable wake-on-lan for r8a7740/armadillo
From: Geert Uytterhoeven @ 2016-12-13 12:51 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Sergei Shtylyov, Simon Horman, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161212160931.6478-4-niklas.soderlund+renesas@ragnatech.se>
On Mon, Dec 12, 2016 at 5:09 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Geert Uytterhoeven reported WoL worked on his Armadillo board.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCHv2 2/5] sh_eth: enable wake-on-lan for Gen2 devices
From: Geert Uytterhoeven @ 2016-12-13 12:52 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Sergei Shtylyov, Simon Horman, netdev@vger.kernel.org,
Linux-Renesas
In-Reply-To: <20161212160931.6478-3-niklas.soderlund+renesas@ragnatech.se>
On Mon, Dec 12, 2016 at 5:09 PM, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Tested on Gen2 r8a7791/Koelsch.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Synopsys Ethernet QoS
From: Joao Pinto @ 2016-12-13 12:56 UTC (permalink / raw)
To: Lars Persson, Niklas Cassel
Cc: Joao Pinto, Florian Fainelli, Andy Shevchenko, David Miller,
Giuseppe CAVALLARO, Rabin Vincent, netdev,
CARLOS.PALMINHA@synopsys.com, Jie.Deng1@synopsys.com,
Stephen Warren, pavel@ucw.cz
In-Reply-To: <46E178AA-6100-4244-BB8D-B5EA58E96E72@axis.com>
Às 12:50 PM de 12/13/2016, Lars Persson escreveu:
>
>> 13 dec. 2016 kl. 13:31 skrev Niklas Cassel <Niklas.Cassel@axis.com>:
>>
>>
>>
>>> On 12/13/2016 12:49 PM, Joao Pinto wrote:
>>> Hi Niklas,
>>>
>>> Às 4:25 PM de 12/12/2016, Niklas Cassel escreveu:
>>>>
>>>>> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>>>>> Hi,
>>>>>
>>>>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>>>>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> (snip...)
>>>
>>>
>>>>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>>>>> driver would it be possible for you to make an initial analysis of what has to
>>>>> be merged into Stmmac? This way the development would speed-up.
>>>> I can answer that question.
>>>>
>>>> I've sent out 12 patches to the stmmac driver
>>>> (all patches are included in the current net-next tree),
>>>> with these patches the stmmac driver works properly on Axis hardware
>>>> (we use Synopsys GMAC 4.10a synthesized with multiple TX queues).
>>>> stmmac's DT binding has also been extended with properties that
>>>> existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.
>>>>
>>>> Since we have no problem updating the DTB together with the kernel,
>>>> we will simply move to using the start using the stmmac driver,
>>>> with stmmac's DT binding.
>>>>
>>>> However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
>>>> I don't how easy it would be for them to switch to stmmac's DT binding.
>>>> (Adding Stephen Warren to CC.)
>>>>
>>>> The reset sequence that Lars Persson was worried about is not an issue
>>>> with the stmmac driver.
>>> Great! So you saying that stmmac works great with AXIS hardware and no need to
>>> merge specific code from AXIS' *qos* driver?
>>
>> Yes. From Axis point of view, we are done.
>> stmmac works and we will move to that driver + DT binding.
>>
>> However, it seems like Stephen Warren will NAK if you try to remove
>> synopsys/dwc_eth_qos.c before
>> Documentation/devicetree/bindings/net/stmmac.txt
>> is compatible with
>> Documentation/devicetree/bindings/net/snps,dwc-qos-ethernet.txt
>>
>> You might want to sync with him. I have no idea, but perhaps they are
>> only using a subset of all the available properties. Perhaps,
>> only implementing what they are using might be enough, I don't know.
>> I couldn't find their DTS in arch/arm/dts.
>> I guess you might want to know David Miller's opinion,
>> since he's the one who decides in the end.
>
> I will also NACK removal of dwc_eth_qos.c until the binding is implemented elsewhere.
Totally agree.
@Niklas: David Miller is informed of what we are planning to do. Can you
estimate the effort of merging the axis driver device tree bindings? If there
was anyone from axis to do the merge would be better since you are familiar with
it. What do you think?
>
>
>>
>>>>
>>>>
>>>>
>>>> There are some performance problems with the stmmac driver though:
>>>>
>>>> When running iperf3 with 3 streams:
>>>> iperf3 -c 192.168.0.90 -P 3 -t 30
>>>> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>>>>
>>>> I get really bad fairness between the streams.
>>>>
>>>> This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
>>>> Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.
>>>> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
>>>> and we don't see the same problem.
>>>>
>>>> Also netperf TCP_RR and UDP_RR gives really bad results compared to the
>>>> dwceqos driver (without IRQ coalescing).
>>>> 2000 transactions/sec vs 9000 transactions/sec.
>>>> Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
>>>> gives the same performance. I guess it's a trade off, low CPU usage
>>>> vs low latency, so I don't know how important TCP_RR/UDP_RR really is.
>>>>
>>>> The best thing would be to get a good working TX IRQ coalesce
>>>> implementation with HR timers in stmmac.
>>>> Perhaps it should also be investigated if the RX interrupt watchdog
>>>> timeout should have a lower default value.
>>>>
>>>>
>>>>
>>>>> Thanks to all.
>>>>>
>>>>> Joao
>>
^ 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