* Re: [PATCH] net: fec: Fixed panic problem with non-tso
From: Eric Dumazet @ 2017-01-19 13:24 UTC (permalink / raw)
To: Andy Duan; +Cc: Ashizuka, Yuusuke, netdev@vger.kernel.org, Pravin B Shelar
In-Reply-To: <AM4PR0401MB2260F19C276A1329FD2BB8A7FF7E0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
On Thu, 2017-01-19 at 08:18 +0000, Andy Duan wrote:
> I will double check your fix. Thanks.
>
> And, if driver is to support highmem, then we should add tso highmem
> support in net/core/tso.c, do you think it is necessary ?
Adding TSO highmem support would mean changing net/core/tso.c ABI and
thus changing all net/core/tso.c users.
Looks a lot of work to me.
^ permalink raw reply
* Re: [PATCH iproute2 net-next V2] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:14 UTC (permalink / raw)
To: Jiri Benc
Cc: paulb, Stephen Hemminger, netdev, Jiri Pirko, Or Gerlitz,
Roi Dayan, Simon Horman
In-Reply-To: <20170119150447.6b9ad397@griffin>
On 19/01/2017 16:04, Jiri Benc wrote:
> On Thu, 19 Jan 2017 14:17:57 +0200, Paul Blakey wrote:
>> Instead of "magic numbers" we can now specify each flag
>> by name. Prefix of "no_" (e.g no_frag) unsets the flag,
>> otherwise it wil be set.
>
> Other parts of the tc tool use the "no" prefix (without the
> underscore), such as "nofrag" in u32 and pedit, "noecn" in fq_codel and
> pie, etc. We should follow the suit here.
>
> Looks good to me otherwise, thanks a lot for doing the work!
>
> Jiri
>
Thanks, v3 is up.
^ permalink raw reply
* Re: [PATCH iproute2 net-next V3] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:19 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: paulb, Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman
In-Reply-To: <1484835241-45689-1-git-send-email-paulb@mellanox.com>
On 19/01/2017 16:14, Paul Blakey wrote:
> Instead of "magic numbers" we can now specify each flag
> by name. Prefix of "no" (e.g nofrag) unsets the flag,
> otherwise it wil be set.
>
> Example:
> # add a flower filter that will drop fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags frag \
> action drop
>
> # add a flower filter that will drop non-fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags nofrag \
> action drop
>
> Fixes: 22a8f019891c ('tc: flower: support matching flags')
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
>
> Hi,
> Added a framework to add new flags more easily, such
> as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
>
> Thanks,
> Paul.
>
>
> Changelog:
>
> v3:
> Changed prefix to "no" instead of "no_".
>
> v2:
> Changed delimiter to "/" to avoid shell pipe errors.
>
>
> man/man8/tc-flower.8 | 12 +++++-
> tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 102 insertions(+), 27 deletions(-)
>
> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
> index 2dd2c5e..77da10b 100644
> --- a/man/man8/tc-flower.8
> +++ b/man/man8/tc-flower.8
> @@ -46,7 +46,9 @@ flower \- flow based traffic control filter
> .BR enc_dst_ip " | " enc_src_ip " } { "
> .IR ipv4_address " | " ipv6_address " } | "
> .B enc_dst_port
> -.IR port_number
> +.IR port_number " | "
> +.BR ip_flags
> +.IR IP_FLAGS
> .SH DESCRIPTION
> The
> .B flower
> @@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
> host match. Dst port
> .I NUMBER
> is a 16 bit UDP dst port.
> +.TP
> +.BI ip_flags " IP_FLAGS"
> +.I IP_FLAGS
> +may be either
> +.BR frag " or " no_frag
> +to match on fragmented packets or not respectively.
> .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
> (\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
> have no dependency, layer three matches
> (\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
> -\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
> +\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
> depend on the
> .B protocol
> option of tc filter, layer four port matches
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index d301db3..3dffe2b 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -24,6 +24,10 @@
> #include "tc_util.h"
> #include "rt_names.h"
>
> +enum flower_matching_flags {
> + FLOWER_IP_FLAGS,
> +};
> +
> enum flower_endpoint {
> FLOWER_ENDPOINT_SRC,
> FLOWER_ENDPOINT_DST
> @@ -63,7 +67,7 @@ static void explain(void)
> " enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_key_id [ KEY-ID ] |\n"
> - " matching_flags MATCHING-FLAGS | \n"
> + " ip_flags IP-FLAGS | \n"
> " enc_dst_port [ port_number ] }\n"
> " FILTERID := X:Y:Z\n"
> " MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
> @@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
> return 0;
> }
>
> -static int flower_parse_matching_flags(char *str, int type, int mask_type,
> - struct nlmsghdr *n)
> -{
> - __u32 mtf, mtf_mask;
> - char *c;
> +struct flag_to_string {
> + int flag;
> + enum flower_matching_flags type;
> + char *string;
> +};
>
> - c = strchr(str, '/');
> - if (c)
> - *c = '\0';
> +static struct flag_to_string flags_str[] = {
> + { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
> +};
>
> - if (get_u32(&mtf, str, 0))
> - return -1;
> +static int flower_parse_matching_flags(char *str,
> + enum flower_matching_flags type,
> + __u32 *mtf, __u32 *mtf_mask)
> +{
> + char *token;
> + bool no;
> + bool found;
> + int i;
>
> - if (c) {
> - if (get_u32(&mtf_mask, ++c, 0))
> + token = strtok(str, "/");
> +
> + while (token) {
> + if (!strncmp(token, "no", 2)) {
> + no = true;
> + token = strchr(token, '_') + 1;
> + } else
> + no = false;
> +
> + found = false;
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> +
> + if (!strcmp(token, flags_str[i].string)) {
> + if (no)
> + *mtf &= ~flags_str[i].flag;
> + else
> + *mtf |= flags_str[i].flag;
> +
> + *mtf_mask |= flags_str[i].flag;
> + found = true;
> + break;
> + }
> + }
> + if (!found)
> return -1;
> - } else {
> - mtf_mask = 0xffffffff;
> +
> + token = strtok(NULL, "/");
> }
>
> - addattr32(n, MAX_MSG, type, htonl(mtf));
> - addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
> return 0;
> }
>
> @@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> __be16 vlan_ethtype = 0;
> __u8 ip_proto = 0xff;
> __u32 flags = 0;
> + __u32 mtf = 0;
> + __u32 mtf_mask = 0;
>
> if (handle) {
> ret = get_u32(&t->tcm_handle, handle, 0);
> @@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> return -1;
> }
> addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
> - } else if (matches(*argv, "matching_flags") == 0) {
> + } else if (matches(*argv, "ip_flags") == 0) {
> NEXT_ARG();
> ret = flower_parse_matching_flags(*argv,
> - TCA_FLOWER_KEY_FLAGS,
> - TCA_FLOWER_KEY_FLAGS_MASK,
> - n);
> + FLOWER_IP_FLAGS,
> + &mtf,
> + &mtf_mask);
> if (ret < 0) {
> - fprintf(stderr, "Illegal \"matching_flags\"\n");
> + fprintf(stderr, "Illegal \"ip_flags\"\n");
> return -1;
> }
> } else if (matches(*argv, "skip_hw") == 0) {
> @@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> parse_done:
> addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
>
> + if (mtf_mask) {
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
> + if (ret)
> + return ret;
> +
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
> + if (ret)
> + return ret;
> + }
> +
> ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
> if (ret) {
> fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
> @@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
> }
>
> static void flower_print_matching_flags(FILE *f, char *name,
> + enum flower_matching_flags type,
> struct rtattr *attr,
> struct rtattr *mask_attr)
> {
> + int i;
> + int count = 0;
> + __u32 mtf;
> + __u32 mtf_mask;
> +
> if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
> return;
>
> - fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
> - mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
> + mtf = ntohl(rta_getattr_u32(attr));
> + mtf_mask = ntohl(rta_getattr_u32(mask_attr));
> +
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> + if (mtf_mask & flags_str[i].flag) {
> + if (++count == 1)
> + fprintf(f, "\n %s ", name);
> + else
> + fprintf(f, "/");
> +
> + if (mtf & flags_str[i].flag)
> + fprintf(f, "%s", flags_str[i].string);
> + else
> + fprintf(f, "no%s", flags_str[i].string);
> + }
> + }
> }
>
> static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
> @@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
> flower_print_port(f, "enc_dst_port",
> tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
>
> - flower_print_matching_flags(f, "matching_flags",
> + flower_print_matching_flags(f, "ip_flags",
> + FLOWER_IP_FLAGS,
> tb[TCA_FLOWER_KEY_FLAGS],
> tb[TCA_FLOWER_KEY_FLAGS_MASK]);
>
>
Forgot to update man page, v4 is up.
^ permalink raw reply
* RE: [PATCH net] gianfar: Do not reuse pages from emergency reserve
From: Claudiu Manoil @ 2017-01-19 14:04 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev
In-Reply-To: <1484797482.16328.13.camel@edumazet-glaptop3.roam.corp.google.com>
>-----Original Message-----
>From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
>Sent: Thursday, January 19, 2017 5:45 AM
>To: David Miller <davem@davemloft.net>
>Cc: netdev <netdev@vger.kernel.org>; Claudiu Manoil
><claudiu.manoil@freescale.com>
>Subject: [PATCH net] gianfar: Do not reuse pages from emergency reserve
>
>From: Eric Dumazet <edumazet@google.com>
>
>A driver using dev_alloc_page() must not reuse a page that had to
>use emergency memory reserve.
>
>Otherwise all packets using this page will be immediately dropped,
>unless for very specific sockets having SOCK_MEMALLOC bit set.
>
>This issue might be hard to debug, because only a fraction of the RX
>ring buffer would suffer from drops.
>
>Fixes: 75354148ce69 ("gianfar: Add paged allocation and Rx S/G")
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Claudiu Manoil <claudiu.manoil@freescale.com>
Thanks.
Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
^ permalink raw reply
* [PATCH iproute2 net-next V3] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:14 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman,
Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no" (e.g nofrag) unsets the flag,
otherwise it wil be set.
Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop
# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags nofrag \
action drop
Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
Thanks,
Paul.
Changelog:
v3:
Changed prefix to "no" instead of "no_".
v2:
Changed delimiter to "/" to avoid shell pipe errors.
man/man8/tc-flower.8 | 12 +++++-
tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..77da10b 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
.SH DESCRIPTION
The
.B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " no_frag
+to match on fragmented packets or not respectively.
.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
(\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
have no dependency, layer three matches
(\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
depend on the
.B protocol
option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..3dffe2b 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
#include "tc_util.h"
#include "rt_names.h"
+enum flower_matching_flags {
+ FLOWER_IP_FLAGS,
+};
+
enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
- " matching_flags MATCHING-FLAGS | \n"
+ " ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
" MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
return 0;
}
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
- struct nlmsghdr *n)
-{
- __u32 mtf, mtf_mask;
- char *c;
+struct flag_to_string {
+ int flag;
+ enum flower_matching_flags type;
+ char *string;
+};
- c = strchr(str, '/');
- if (c)
- *c = '\0';
+static struct flag_to_string flags_str[] = {
+ { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
- if (get_u32(&mtf, str, 0))
- return -1;
+static int flower_parse_matching_flags(char *str,
+ enum flower_matching_flags type,
+ __u32 *mtf, __u32 *mtf_mask)
+{
+ char *token;
+ bool no;
+ bool found;
+ int i;
- if (c) {
- if (get_u32(&mtf_mask, ++c, 0))
+ token = strtok(str, "/");
+
+ while (token) {
+ if (!strncmp(token, "no", 2)) {
+ no = true;
+ token = strchr(token, '_') + 1;
+ } else
+ no = false;
+
+ found = false;
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+
+ if (!strcmp(token, flags_str[i].string)) {
+ if (no)
+ *mtf &= ~flags_str[i].flag;
+ else
+ *mtf |= flags_str[i].flag;
+
+ *mtf_mask |= flags_str[i].flag;
+ found = true;
+ break;
+ }
+ }
+ if (!found)
return -1;
- } else {
- mtf_mask = 0xffffffff;
+
+ token = strtok(NULL, "/");
}
- addattr32(n, MAX_MSG, type, htonl(mtf));
- addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
return 0;
}
@@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
__be16 vlan_ethtype = 0;
__u8 ip_proto = 0xff;
__u32 flags = 0;
+ __u32 mtf = 0;
+ __u32 mtf_mask = 0;
if (handle) {
ret = get_u32(&t->tcm_handle, handle, 0);
@@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
- } else if (matches(*argv, "matching_flags") == 0) {
+ } else if (matches(*argv, "ip_flags") == 0) {
NEXT_ARG();
ret = flower_parse_matching_flags(*argv,
- TCA_FLOWER_KEY_FLAGS,
- TCA_FLOWER_KEY_FLAGS_MASK,
- n);
+ FLOWER_IP_FLAGS,
+ &mtf,
+ &mtf_mask);
if (ret < 0) {
- fprintf(stderr, "Illegal \"matching_flags\"\n");
+ fprintf(stderr, "Illegal \"ip_flags\"\n");
return -1;
}
} else if (matches(*argv, "skip_hw") == 0) {
@@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
parse_done:
addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (mtf_mask) {
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
+ if (ret)
+ return ret;
+
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
+ if (ret)
+ return ret;
+ }
+
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
@@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
}
static void flower_print_matching_flags(FILE *f, char *name,
+ enum flower_matching_flags type,
struct rtattr *attr,
struct rtattr *mask_attr)
{
+ int i;
+ int count = 0;
+ __u32 mtf;
+ __u32 mtf_mask;
+
if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
return;
- fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
- mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
+ mtf = ntohl(rta_getattr_u32(attr));
+ mtf_mask = ntohl(rta_getattr_u32(mask_attr));
+
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+ if (mtf_mask & flags_str[i].flag) {
+ if (++count == 1)
+ fprintf(f, "\n %s ", name);
+ else
+ fprintf(f, "/");
+
+ if (mtf & flags_str[i].flag)
+ fprintf(f, "%s", flags_str[i].string);
+ else
+ fprintf(f, "no%s", flags_str[i].string);
+ }
+ }
}
static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
@@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port(f, "enc_dst_port",
tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
- flower_print_matching_flags(f, "matching_flags",
+ flower_print_matching_flags(f, "ip_flags",
+ FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next V4] tc: flower: Refactor matching flags to be more user friendly
From: Jiri Benc @ 2017-01-19 14:22 UTC (permalink / raw)
To: Paul Blakey
Cc: Stephen Hemminger, netdev, Jiri Pirko, Or Gerlitz, Roi Dayan,
Simon Horman
In-Reply-To: <1484835468-46051-1-git-send-email-paulb@mellanox.com>
On Thu, 19 Jan 2017 16:17:48 +0200, Paul Blakey wrote:
> + while (token) {
> + if (!strncmp(token, "no", 2)) {
> + no = true;
> + token = strchr(token, '_') + 1;
This seems to still assume that "no" is followed by an underscore.
What about a simple token += 2?
Jiri
^ permalink raw reply
* [PATCH net-next V4] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:17 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman,
Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no" (e.g nofrag) unsets the flag,
otherwise it wil be set.
Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop
# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags nofrag \
action drop
Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
Thanks,
Paul.
Changelog:
v4:
Changed prefix in manpage as well.
v3:
Changed prefix to "no" instead of "no_".
v2:
Changed delimiter to "/" to avoid shell pipe errors.
man/man8/tc-flower.8 | 12 +++++-
tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..2bdd2ef 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
.SH DESCRIPTION
The
.B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " nofrag
+to match on fragmented packets or not respectively.
.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
(\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
have no dependency, layer three matches
(\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
depend on the
.B protocol
option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..3dffe2b 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
#include "tc_util.h"
#include "rt_names.h"
+enum flower_matching_flags {
+ FLOWER_IP_FLAGS,
+};
+
enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
- " matching_flags MATCHING-FLAGS | \n"
+ " ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
" MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
return 0;
}
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
- struct nlmsghdr *n)
-{
- __u32 mtf, mtf_mask;
- char *c;
+struct flag_to_string {
+ int flag;
+ enum flower_matching_flags type;
+ char *string;
+};
- c = strchr(str, '/');
- if (c)
- *c = '\0';
+static struct flag_to_string flags_str[] = {
+ { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
- if (get_u32(&mtf, str, 0))
- return -1;
+static int flower_parse_matching_flags(char *str,
+ enum flower_matching_flags type,
+ __u32 *mtf, __u32 *mtf_mask)
+{
+ char *token;
+ bool no;
+ bool found;
+ int i;
- if (c) {
- if (get_u32(&mtf_mask, ++c, 0))
+ token = strtok(str, "/");
+
+ while (token) {
+ if (!strncmp(token, "no", 2)) {
+ no = true;
+ token = strchr(token, '_') + 1;
+ } else
+ no = false;
+
+ found = false;
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+
+ if (!strcmp(token, flags_str[i].string)) {
+ if (no)
+ *mtf &= ~flags_str[i].flag;
+ else
+ *mtf |= flags_str[i].flag;
+
+ *mtf_mask |= flags_str[i].flag;
+ found = true;
+ break;
+ }
+ }
+ if (!found)
return -1;
- } else {
- mtf_mask = 0xffffffff;
+
+ token = strtok(NULL, "/");
}
- addattr32(n, MAX_MSG, type, htonl(mtf));
- addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
return 0;
}
@@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
__be16 vlan_ethtype = 0;
__u8 ip_proto = 0xff;
__u32 flags = 0;
+ __u32 mtf = 0;
+ __u32 mtf_mask = 0;
if (handle) {
ret = get_u32(&t->tcm_handle, handle, 0);
@@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
- } else if (matches(*argv, "matching_flags") == 0) {
+ } else if (matches(*argv, "ip_flags") == 0) {
NEXT_ARG();
ret = flower_parse_matching_flags(*argv,
- TCA_FLOWER_KEY_FLAGS,
- TCA_FLOWER_KEY_FLAGS_MASK,
- n);
+ FLOWER_IP_FLAGS,
+ &mtf,
+ &mtf_mask);
if (ret < 0) {
- fprintf(stderr, "Illegal \"matching_flags\"\n");
+ fprintf(stderr, "Illegal \"ip_flags\"\n");
return -1;
}
} else if (matches(*argv, "skip_hw") == 0) {
@@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
parse_done:
addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (mtf_mask) {
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
+ if (ret)
+ return ret;
+
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
+ if (ret)
+ return ret;
+ }
+
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
@@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
}
static void flower_print_matching_flags(FILE *f, char *name,
+ enum flower_matching_flags type,
struct rtattr *attr,
struct rtattr *mask_attr)
{
+ int i;
+ int count = 0;
+ __u32 mtf;
+ __u32 mtf_mask;
+
if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
return;
- fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
- mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
+ mtf = ntohl(rta_getattr_u32(attr));
+ mtf_mask = ntohl(rta_getattr_u32(mask_attr));
+
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+ if (mtf_mask & flags_str[i].flag) {
+ if (++count == 1)
+ fprintf(f, "\n %s ", name);
+ else
+ fprintf(f, "/");
+
+ if (mtf & flags_str[i].flag)
+ fprintf(f, "%s", flags_str[i].string);
+ else
+ fprintf(f, "no%s", flags_str[i].string);
+ }
+ }
}
static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
@@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port(f, "enc_dst_port",
tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
- flower_print_matching_flags(f, "matching_flags",
+ flower_print_matching_flags(f, "ip_flags",
+ FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Greg KH @ 2017-01-19 14:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Jason Cooper, Vivien Didelot, netdev, Russell King,
open list, Gregory Clement, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, Sebastian Hesselbarth
In-Reply-To: <912e6143-c67a-d909-2a6c-939de79efc5d@gmail.com>
On Mon, Jan 16, 2017 at 12:01:02PM -0800, Florian Fainelli wrote:
> On 01/15/2017 11:16 AM, Andrew Lunn wrote:
> >>> What exactly is the relationship between these devices (a ascii-art tree
> >>> or sysfs tree output might be nice) so I can try to understand what is
> >>> going on here.
> >
> > Hi Greg, Florian
> >
> > A few diagrams and trees which might help understand what is going on.
> >
> > The first diagram comes from the 2008 patch which added all this code:
> >
> > +-----------+ +-----------+
> > | | RGMII | |
> > | +-------+ +------ 1000baseT MDI ("WAN")
> > | | | 6-port +------ 1000baseT MDI ("LAN1")
> > | CPU | | ethernet +------ 1000baseT MDI ("LAN2")
> > | |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
> > | +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
> > | | | |
> > +-----------+ +-----------+
> >
> > We have an ethernet switch and a host CPU. The switch is connected to
> > the CPU in two different ways. RGMII allows us to get Ethernet frames
> > from the CPU into the switch. MIImgmt, is the management bus normally
> > used for Ethernet PHYs, but Marvell switches also use it for Managing
> > switches.
> >
> > The diagram above is the simplest setup. You can have multiple
> > Ethernet switches, connected together via switch ports. Each switch
> > has its own MIImgmt connect to the CPU, but there is only one RGMII
> > link.
> >
> > When this code was designed back in 2008, it was decided to represent
> > this is a platform device, and it has a platform_data, which i have
> > slightly edited to keep it simple:
> >
> > struct dsa_platform_data {
> > /*
> > * Reference to a Linux network interface that connects
> > * to the root switch chip of the tree.
> > */
> > struct device *netdev;
This I think is the oddest thing, why do you need to have the "root
switch" here? You seem to have dropped the next value in this
structure:
struct net_device *of_netdev;
Isn't that the "real" net_device you need/want to get to here?
Or, when you set netdev, can't you also point to the "real" net_device
you want to later get to?
> > /*
> > * Info structs describing each of the switch chips
> > * connected via this network interface.
> > */
> > int nr_chips;
> > struct dsa_chip_data *chip;
> > };
> >
> > This netdev is the CPU side of the RGMII interface.
> >
> > Each switch has a dsa_chip_data, again edited:
> >
> > struct dsa_chip_data {
> > /*
> > * How to access the switch configuration registers.
> > */
> > struct device *host_dev;
> > int sw_addr;
> > ...
> > }
If each switch only has one dsa_chip_data, can't you point directly from
that structure back to the dsa_platform_device? Or is that what this
"host_dev" pointer is pointing to?
> > The host_dev is the CPU side of the MIImgmt, and we have the address
> > the switch is using on the bus.
"cpu side" means what? What 'struct device' is this? The host
controller? The platform device? Something else?
> > During probe of this platform device, we need to get from the
> > struct device *netdev to a struct net_device *dev.
Wait, what exactly is this "struct device *"? Who created it?
And why are you using a platform device? Shouldn't this be a custom
bus? I know people like to abuse platform devices, is that the case
here, or is it really driven only by a device tree representation?
Shouldn't you have a bus for RGMII devices? Is that the real problem
here, you don't have a representation for your RGMII "bus" with a
controller to bundle everything under (like a USB host controller, it
bridges from one bus to another).
> > So the code looks in the device net class to find the device
> >
> > | | | |-- f1074000.ethernet
> > | | | | |-- deferred_probe
> > | | | | |-- driver -> ../../../../../bus/platform/drivers/mvneta
> > | | | | |-- driver_override
> > | | | | |-- modalias
> > | | | | |-- net
> > | | | | | `-- eth1
> > | | | | | |-- addr_assign_type
> > | | | | | |-- address
> > | | | | | |-- addr_len
> > | | | | | |-- broadcast
> > | | | | | |-- carrier
> > | | | | | |-- carrier_changes
> > | | | | | |-- deferred_probe
> > | | | | | |-- device -> ../../../f1074000.ethernet
> >
> > and then use container_of() to get the net_device.
What 'struct device *' are you passing to container_of() here? The one
representing eth1? What call is this? And where are you trying to go
from there, to a peer? Why?
> > Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
> >
> > | | | |-- f1072004.mdio
> > | | | | |-- deferred_probe
> > | | | | |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
> > | | | | |-- driver_override
> > | | | | |-- mdio_bus
> > | | | | | `-- f1072004.mdio-mi
> > | | | | | |-- deferred_probe
> > | | | | | |-- device -> ../../../f1072004.mdio
Ok, this looks like the "peer" you want to get to from eth1, you want
f1072004.mdio-mi, right?
If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?
Why is it up attached to the parent device, making this a "flat"
heirachy?
That's what is confusing about your functions, you are just walking all
of the "child" devices of a specific struct device, trying to find the
first random one that happens to belong to a specific 'bus' because you
are related to it.
Which is wrong, eth1 should be a child of your bus device, that way you
"know" how to get to it (it's your parent!).
So, I'm still confused, and still think this is all wrong, but feel free
to prove me wrong about this :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH iproute2 net-next V5] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:27 UTC (permalink / raw)
To: Stephen Hemminger, netdev
Cc: Jiri Pirko, Or Gerlitz, Roi Dayan, Jiri Benc, Simon Horman,
Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no" (e.g nofrag) unsets the flag,
otherwise it wil be set.
Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop
# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent ffff: \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags nofrag \
action drop
Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
Thanks,
Paul.
Changelog:
v5:
Fixed wrong use of strtok to skip old prefix.
v4:
Changed prefix in manpage as well.
v3:
Changed prefix to "no" instead of "no_".
v2:
Changed delimiter to "/" to avoid shell pipe errors.
man/man8/tc-flower.8 | 12 +++++-
tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 102 insertions(+), 27 deletions(-)
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..2bdd2ef 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
.BR enc_dst_ip " | " enc_src_ip " } { "
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
.SH DESCRIPTION
The
.B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " nofrag
+to match on fragmented packets or not respectively.
.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
(\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
have no dependency, layer three matches
(\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
depend on the
.B protocol
option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..b42e529 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
#include "tc_util.h"
#include "rt_names.h"
+enum flower_matching_flags {
+ FLOWER_IP_FLAGS,
+};
+
enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
- " matching_flags MATCHING-FLAGS | \n"
+ " ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
" MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
return 0;
}
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
- struct nlmsghdr *n)
-{
- __u32 mtf, mtf_mask;
- char *c;
+struct flag_to_string {
+ int flag;
+ enum flower_matching_flags type;
+ char *string;
+};
- c = strchr(str, '/');
- if (c)
- *c = '\0';
+static struct flag_to_string flags_str[] = {
+ { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
- if (get_u32(&mtf, str, 0))
- return -1;
+static int flower_parse_matching_flags(char *str,
+ enum flower_matching_flags type,
+ __u32 *mtf, __u32 *mtf_mask)
+{
+ char *token;
+ bool no;
+ bool found;
+ int i;
- if (c) {
- if (get_u32(&mtf_mask, ++c, 0))
+ token = strtok(str, "/");
+
+ while (token) {
+ if (!strncmp(token, "no", 2)) {
+ no = true;
+ token += 2;
+ } else
+ no = false;
+
+ found = false;
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+
+ if (!strcmp(token, flags_str[i].string)) {
+ if (no)
+ *mtf &= ~flags_str[i].flag;
+ else
+ *mtf |= flags_str[i].flag;
+
+ *mtf_mask |= flags_str[i].flag;
+ found = true;
+ break;
+ }
+ }
+ if (!found)
return -1;
- } else {
- mtf_mask = 0xffffffff;
+
+ token = strtok(NULL, "/");
}
- addattr32(n, MAX_MSG, type, htonl(mtf));
- addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
return 0;
}
@@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
__be16 vlan_ethtype = 0;
__u8 ip_proto = 0xff;
__u32 flags = 0;
+ __u32 mtf = 0;
+ __u32 mtf_mask = 0;
if (handle) {
ret = get_u32(&t->tcm_handle, handle, 0);
@@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
return -1;
}
addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
- } else if (matches(*argv, "matching_flags") == 0) {
+ } else if (matches(*argv, "ip_flags") == 0) {
NEXT_ARG();
ret = flower_parse_matching_flags(*argv,
- TCA_FLOWER_KEY_FLAGS,
- TCA_FLOWER_KEY_FLAGS_MASK,
- n);
+ FLOWER_IP_FLAGS,
+ &mtf,
+ &mtf_mask);
if (ret < 0) {
- fprintf(stderr, "Illegal \"matching_flags\"\n");
+ fprintf(stderr, "Illegal \"ip_flags\"\n");
return -1;
}
} else if (matches(*argv, "skip_hw") == 0) {
@@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
parse_done:
addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
+ if (mtf_mask) {
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
+ if (ret)
+ return ret;
+
+ ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
+ if (ret)
+ return ret;
+ }
+
ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
if (ret) {
fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
@@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
}
static void flower_print_matching_flags(FILE *f, char *name,
+ enum flower_matching_flags type,
struct rtattr *attr,
struct rtattr *mask_attr)
{
+ int i;
+ int count = 0;
+ __u32 mtf;
+ __u32 mtf_mask;
+
if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
return;
- fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
- mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
+ mtf = ntohl(rta_getattr_u32(attr));
+ mtf_mask = ntohl(rta_getattr_u32(mask_attr));
+
+ for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+ if (type != flags_str[i].type)
+ continue;
+ if (mtf_mask & flags_str[i].flag) {
+ if (++count == 1)
+ fprintf(f, "\n %s ", name);
+ else
+ fprintf(f, "/");
+
+ if (mtf & flags_str[i].flag)
+ fprintf(f, "%s", flags_str[i].string);
+ else
+ fprintf(f, "no%s", flags_str[i].string);
+ }
+ }
}
static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
@@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port(f, "enc_dst_port",
tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
- flower_print_matching_flags(f, "matching_flags",
+ flower_print_matching_flags(f, "ip_flags",
+ FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] rtlwifi: rtl8192x: Enabling and disabling hardware interrupts after enabling local irq flags
From: Lino Sanfilippo @ 2017-01-19 14:35 UTC (permalink / raw)
To: Bharat Kumar Gogada
Cc: Larry.Finger, chaoming_li, linux-wireless, linux-kernel, kvalo,
netdev, rgummal, Bharat Kumar Gogada
In-Reply-To: <1484820854-16719-1-git-send-email-bharatku@xilinx.com>
Hi,
altek/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
> index a47be73..143766c4 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
> @@ -1306,9 +1306,9 @@ void rtl92ce_enable_interrupt(struct ieee80211_hw *hw)
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
>
> + rtlpci->irq_enabled = true;
> rtl_write_dword(rtlpriv, REG_HIMR, rtlpci->irq_mask[0] & 0xFFFFFFFF);
> rtl_write_dword(rtlpriv, REG_HIMRE, rtlpci->irq_mask[1] & 0xFFFFFFFF);
> - rtlpci->irq_enabled = true;
> }
>
> void rtl92ce_disable_interrupt(struct ieee80211_hw *hw)
> @@ -1316,9 +1316,9 @@ void rtl92ce_disable_interrupt(struct ieee80211_hw *hw)
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
>
> + rtlpci->irq_enabled = false;
> rtl_write_dword(rtlpriv, REG_HIMR, IMR8190_DISABLED);
> rtl_write_dword(rtlpriv, REG_HIMRE, IMR8190_DISABLED);
> - rtlpci->irq_enabled = false;
> }
>
AFAIK you also have to use memory barriers here to ensure that
the concerning instructions are not reordered, and both irq handler
and process have a consistent perception of irq_enabled, e.g:
rtlpci->irq_enabled = true;
smp_wmb();
rtl_write_dword(rtlpriv, REG_HIMR, rtlpci->irq_mask[0] & 0xFFFFFFFF);
and in the irq handler
if (rtlpci->irq_enabled == 0) {
smp_rmb();
return ret;
}
Regards,
Lino
^ permalink raw reply
* Re: [PATCH iproute2 net-next V5] tc: flower: Refactor matching flags to be more user friendly
From: Jiri Benc @ 2017-01-19 14:48 UTC (permalink / raw)
To: Paul Blakey
Cc: Stephen Hemminger, netdev, Jiri Pirko, Or Gerlitz, Roi Dayan,
Simon Horman
In-Reply-To: <1484836073-47019-1-git-send-email-paulb@mellanox.com>
On Thu, 19 Jan 2017 16:27:53 +0200, Paul Blakey wrote:
> Instead of "magic numbers" we can now specify each flag
> by name. Prefix of "no" (e.g nofrag) unsets the flag,
> otherwise it wil be set.
>
> Example:
> # add a flower filter that will drop fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags frag \
> action drop
>
> # add a flower filter that will drop non-fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags nofrag \
> action drop
>
> Fixes: 22a8f019891c ('tc: flower: support matching flags')
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
Reviewed-by: Jiri Benc <jbenc@redhat.com>
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Andrew Lunn @ 2017-01-19 14:53 UTC (permalink / raw)
To: Greg KH
Cc: Florian Fainelli, Jason Cooper, Vivien Didelot, netdev,
Russell King, open list, Gregory Clement, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, Sebastian Hesselbarth
In-Reply-To: <20170119142820.GA494@kroah.com>
> > > struct dsa_platform_data {
> > > /*
> > > * Reference to a Linux network interface that connects
> > > * to the root switch chip of the tree.
> > > */
> > > struct device *netdev;
>
> This I think is the oddest thing, why do you need to have the "root
> switch" here? You seem to have dropped the next value in this
> structure:
> struct net_device *of_netdev;
We are implementing platform_data for devices which don't support
device tree. When using OF, we don't have any of these issues. We can
go straight to the device.
It is a bit convoluted, but look at
arch/arm/mach-orion5x/rd88f5181l-ge-setup.c. It defines the start of
the dsa_platform_data in that file. It then gets passed through
common.c: orion5x_eth_switch_init() to
arch/arm/plat-orion/common.c:orion_ge00_switch_init() :
void __init orion_ge00_switch_init(struct dsa_platform_data *d)
{
int i;
d->netdev = &orion_ge00.dev;
for (i = 0; i < d->nr_chips; i++)
d->chip[i].host_dev = &orion_ge_mvmdio.dev;
platform_device_register_data(NULL, "dsa", 0, d, sizeof(d));
}
Where we have
static struct platform_device orion_ge00 = {
.name = MV643XX_ETH_NAME,
.id = 0,
.num_resources = 1,
.resource = orion_ge00_resources,
.dev = {
.coherent_dma_mask = DMA_BIT_MASK(32),
},
};
So this is the platform device for the Ethernet device. We cannot go
to the net_device, because it does not exist until this Ethernet
platform device is instantiated.
> Shouldn't you have a bus for RGMII devices? Is that the real problem
> here, you don't have a representation for your RGMII "bus" with a
> controller to bundle everything under (like a USB host controller, it
> bridges from one bus to another).
RGMII is not a bus. It is a point to point link. Normally, it is
between the Ethernet MAC and the Ethernet PHY. But you can also have
it between an Ethernet MAC and another Ethernet MAC. I'm not sure
describing this is a bus would be practical. It would mean every
ethernet driver also becomes a bus driver! Every Ethernet PHY would
become a bus device. That is a huge change, for a few legacy boards
which are not getting converted to device tree.
> If so, why is eth1 not below f1072004.mdio-mi in the heirachy already?
See the initial diagram above. The switch has two parents. It hangs of
an MDIO bus, and you would like to make RGMII also a bus. Can the
device model handle that? I thought it was a tree, not a graph?
Andrew
^ permalink raw reply
* Re: [PATCH iproute2 net-next V2] tc: flower: Refactor matching flags to be more user friendly
From: Jiri Benc @ 2017-01-19 14:04 UTC (permalink / raw)
To: Paul Blakey
Cc: Stephen Hemminger, netdev, Jiri Pirko, Or Gerlitz, Roi Dayan,
Simon Horman
In-Reply-To: <1484828277-29178-1-git-send-email-paulb@mellanox.com>
On Thu, 19 Jan 2017 14:17:57 +0200, Paul Blakey wrote:
> Instead of "magic numbers" we can now specify each flag
> by name. Prefix of "no_" (e.g no_frag) unsets the flag,
> otherwise it wil be set.
Other parts of the tc tool use the "no" prefix (without the
underscore), such as "nofrag" in u32 and pedit, "noecn" in fq_codel and
pie, etc. We should follow the suit here.
Looks good to me otherwise, thanks a lot for doing the work!
Jiri
^ permalink raw reply
* Re: [PATCH net-next V4] tc: flower: Refactor matching flags to be more user friendly
From: Paul Blakey @ 2017-01-19 14:29 UTC (permalink / raw)
To: Jiri Benc
Cc: paulb, Stephen Hemminger, netdev, Jiri Pirko, Or Gerlitz,
Roi Dayan, Simon Horman
In-Reply-To: <20170119152218.61aa2fc4@griffin>
On 19/01/2017 16:22, Jiri Benc wrote:
> On Thu, 19 Jan 2017 16:17:48 +0200, Paul Blakey wrote:
>> + while (token) {
>> + if (!strncmp(token, "no", 2)) {
>> + no = true;
>> + token = strchr(token, '_') + 1;
>
> This seems to still assume that "no" is followed by an underscore.
> What about a simple token += 2?
>
> Jiri
>
Thanks again, v5 is sent.
^ permalink raw reply
* Re: [PATCH iproute2 net-next V5] tc: flower: Refactor matching flags to be more user friendly
From: Or Gerlitz @ 2017-01-19 14:53 UTC (permalink / raw)
To: Simon Horman
Cc: Paul Blakey, Stephen Hemminger, netdev, Jiri Pirko, Roi Dayan,
Jiri Benc
In-Reply-To: <1484836073-47019-1-git-send-email-paulb@mellanox.com>
On 1/19/2017 4:27 PM, Paul Blakey wrote:
> Hi, added a framework to add new flags more easily, such
> as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
FWIW, just wanted to make a note that I have the patches for tcp_flags
under the works, so if someone needs them urgently send me email, want
to avoid wasting your time implementing this..
Or.
^ permalink raw reply
* [PATCH net-next] net: caif: Remove unused stats member from struct chnl_net
From: Tobias Klauser @ 2017-01-19 15:04 UTC (permalink / raw)
To: Dmitry Tarnyagin, David S. Miller; +Cc: netdev
The stats member of struct chnl_net is used nowhere in the code, so it
might as well be removed.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
net/caif/chnl_net.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 3408ed51b611..1816fc9f1ee7 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -44,7 +44,6 @@ enum caif_states {
struct chnl_net {
struct cflayer chnl;
- struct net_device_stats stats;
struct caif_connect_request conn_req;
struct list_head list_field;
struct net_device *netdev;
--
2.11.0
^ permalink raw reply related
* Re: Initializing MAC address at run-time
From: Mason @ 2017-01-19 15:31 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Mark Rutland, DT, Arnd Bergmann, Kevin Hilman, netdev,
Thibaud Cornic, Linux ARM
In-Reply-To: <20170118185435.fen6np7lydzgxxek@pengutronix.de>
Hello Uwe,
On 18/01/2017 19:54, Uwe Kleine-König wrote:
> On Wed, Jan 18, 2017 at 03:03:57PM +0100, Mason wrote:
>
>> When my system boots up, eth0 is given a seemingly random MAC address.
>>
>> [ 0.950734] nb8800 26000.ethernet eth0: MAC address ba:de:d6:38:b8:38
>> [ 0.957334] nb8800 26000.ethernet eth0: MAC address 6e:f1:48:de:d6:c4
>>
>>
>> The DT node for eth0 is:
>>
>> eth0: ethernet@26000 {
>> compatible = "sigma,smp8734-ethernet";
>> reg = <0x26000 0x800>;
>> interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&clkgen SYS_CLK>;
>> };
>>
>> Documentation/devicetree/bindings/net/ethernet.txt mentions
>> - local-mac-address: array of 6 bytes, specifies the MAC address that was
>> assigned to the network device;
>>
>> And indeed, if I define this property, eth0 ends up with the MAC address
>> I specify in the device tree. But of course, I don't want all my boards
>> to share the same MAC address. Every interface has a unique MAC address.
>>
>> In fact, the boot loader (not Uboot, a custom non-DT boot loader) stores
>> the MAC address somewhere in MMIO space, in some weird custom format.
>
> Where does your machine get the dtb from? You write it to the boot
> medium at a certain point of time I assume.
The DTB is appended to the kernel uImage.
(It's never written to the board's storage.)
> So AFAICT you have the following options (in no particular order):
>
> a) Describe in the dtb how to find out how the MAC address is stored
> (already pointed out Mark and Robin)
> b) Make your bootloader dt aware and let it provide the
> local-mac-address property.
Do you agree that such boot loader would execute code that is roughly
identical to the one posted for illustration purposes?
1. find the MAC address to use for eth0
2. find the eth0 node in the DT
3. insert the right prop in the eth0 node
In which case, it seems a waste to add the DT library to the boot
loader, when the operation can be done in Linux, which requires the
DT library anyway. (Additionally, adding DT support to some custom
legacy boot loader might be a complex task.)
> c) Adapt the dtb before it is written to the boot medium.
This is not applicable, as the DTB is not written to the board.
> d) Let the bootloader configure the device and teach the driver to pick
> up the mac from the device's address space.
I'm not sure what you call "the device" ?
The local RAM where the MAC is stored? the eth controller?
> e) Accept that the mac address is random during development, and make
> Userspace configure the MAC address, which is early enough for
> production use.
During development, some devs configure the DHCP server to provide
a specific uImage and/or rootfs to their board, based on the MAC
address. This scheme would fall apart with a random MAC.
> Not sure d) is considered ok today, but some drivers have this feature.
> I'd say b) is the best choice.
In my mind, doing it early in Linux is similar in spirit to doing it
at the boot loader stage, in that it's neatly separated from the rest
of the setup.
>> I need to do something similar with the NAND partitions. The boot loader
>> stores the partition offsets somewhere, and I need to pass this info
>> to the NAND framework, so I assumed that inserting the corresponding
>> properties at run-time was the correct way to do it.
>
> The list of options here is similar to the list above. d) doesn't work,
> but instead you can pass the partitioning on the kernel commandline.
Yes, I will test the command line approach. Thanks.
Regards.
^ permalink raw reply
* Re: [PATCH] net: ethtool: avoid allocation failure for dump_regs
From: John W. Linville @ 2017-01-19 14:15 UTC (permalink / raw)
To: David Arcari; +Cc: David Miller, netdev
In-Reply-To: <e88b01a6-dae8-a97e-9a8b-8aab030ad099@redhat.com>
On Thu, Jan 19, 2017 at 07:35:22AM -0500, David Arcari wrote:
> On 01/18/2017 11:45 AM, David Miller wrote:
> > From: David Arcari <darcari@redhat.com>
> > Date: Wed, 18 Jan 2017 08:34:05 -0500
> >
> >> If the user executes 'ethtool -d' for an interface and the associated
> >> get_regs_len() function returns 0, the user will see a call trace from
> >> the vmalloc() call in ethtool_get_regs(). This patch modifies
> >> ethtool_get_regs() to avoid the call to vmalloc when the size is zero.
> >>
> >> Signed-off-by: David Arcari <darcari@redhat.com>
> > I think when the driver indicates this, it is equivalent to saying that
> > the operation isn't supported.
> >
> > Also, this guards us against ->get_regs() methods that don't handle
> > zero length requests properly. I see many which are going to do
> > really terrible things in that situation.
> >
> > Therefore, if get_regs_len() returns zero, treat it the safe as if the
> > ethtool operations were NULL.
> >
> > Thanks.
>
> That was actually the fix that I was originally considering, but it
> turns out
> there is a problem with it.
>
> I found that the vmalloc error was occurring because
> ieee80211_get_regs_len() in
> net/mac80211/ethtool.c was returning zero. The ieee80211_get_regs in
> the same
> file returns the hw version. It turns out that this information is used
> by the
> at76c50x-usb driver in the user space ethtool to report which HW variant
> is in
> use. Returning an error when regs_len() returns zero would break this
> functionality.
>
> -Dave
I'm responsible for this mess. The original idea was for various
mac80211-based drivers to override the ethtool operation and provide
their own dump operation, but the mac80211 crowd never embraced
the idea.
In the meantime, I added the default implementation which just
passed-up wdev->wiphy->hw_version as the version info for a 0-length
register dump. I then implemented a driver-specific regiser dump
handler for userland ethtool that would interpret the hardware version
information for the at76c50x-usb driver.
So the net of it is, if we treat a return of 0 from get_regs_len()
as "not supported", we break this one driver-specific feature for
userland ethtool. Realistically, there are probably very few users
to care. But I can't guarantee that the number is zero.
Possible solutions:
-- break userland ethtool for at76c50x-usb
-- avoid 0-len allocation attempt (David Arcari's patch)
-- make allocator accept a 0 length value w/o oops'ing
-- change mac8011 code to return non-zero from get_regs_len()
Thoughts? The last option holds a certain attraction, but I'm not
sure how to make it useful...?
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* RE: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics
From: Nogah Frankel @ 2017-01-19 15:21 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev@vger.kernel.org, roszenrami@gmail.com,
roopa@cumulusnetworks.com, Jiri Pirko, Ido Schimmel, Elad Raz,
Yotam Gigi, Or Gerlitz
In-Reply-To: <20170112174335.1bd391c2@xeon-e3>
> -----Original Message-----
> From: Nogah Frankel
> Sent: Sunday, January 15, 2017 3:55 PM
> To: 'Stephen Hemminger' <stephen@networkplumber.org>
> Cc: netdev@vger.kernel.org; roszenrami@gmail.com; roopa@cumulusnetworks.com; Jiri
> Pirko <jiri@mellanox.com>; Ido Schimmel <idosch@mellanox.com>; Elad Raz
> <eladr@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> <ogerlitz@mellanox.com>
> Subject: RE: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics
>
>
>
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, January 13, 2017 3:44 AM
> > To: Nogah Frankel <nogahf@mellanox.com>
> > Cc: netdev@vger.kernel.org; roszenrami@gmail.com; roopa@cumulusnetworks.com;
> Jiri
> > Pirko <jiri@mellanox.com>; Ido Schimmel <idosch@mellanox.com>; Elad Raz
> > <eladr@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> > <ogerlitz@mellanox.com>
> > Subject: Re: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics
> >
> > On Thu, 12 Jan 2017 15:49:50 +0200
> > Nogah Frankel <nogahf@mellanox.com> wrote:
> >
> > > The default stats for ifstat are 32 bits based.
> > > The kernel supports 64 bits based stats. (They are returned in struct
> > > rtnl_link_stats64 which is an exact copy of struct rtnl_link_stats, in
> > > which the "normal" stats are returned, but with fields of u64 instead of
> > > u32). This patch adds them as an extended stats.
> > >
> > > It is read with filter type IFLA_STATS_LINK_64 and no sub type.
> > >
> > > It is under the name 64bits
> > > (or any shorten of it as "64")
> > >
> > > For example:
> > > ifstat -x 64bit
> > >
> > > Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> > > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> >
> > Other commands (like ip link) always use the 64 bit statistics if available
> > from the device. I see no reason that ifstat needs to be different.
> >
>
> Do you mean to change the default ifstat results to be 64 bits based?
> I tried it in the first version, but Roopa commented that it was not a good idea.
> She said they tried it in the past and it caused backward compatibilities problems.
> (Or maybe I didn't understand correctly)
So, can I leave the default ifstat results to be 32 bits based, for the time being?
^ permalink raw reply
* Re: Setting link down or up in software
From: Mason @ 2017-01-19 15:40 UTC (permalink / raw)
To: Zefir Kurtisi, netdev
Cc: Mans Rullgard, Florian Fainelli, Andrew Lunn, Thibaud Cornic
In-Reply-To: <dc6378d7-d370-c893-63b7-c21cd7ef76fd@neratec.com>
On 18/01/2017 11:29, Zefir Kurtisi wrote:
> On 01/13/2017 06:35 PM, Mason wrote:
>> On 13/01/2017 17:28, Zefir Kurtisi wrote:
>>
>>> As for your specific problem: since I fought myself with the PHY/ETH subsystems
>>> over the past months, I might remember something relevant to your issue. Could you
>>> give some more info on your setup (PHY driver, opmode (SGMII, RGMII, etc.), ETH).
>>
>> Hello Zefir,
>>
>> My boards are using these drivers:
>>
>> http://lxr.free-electrons.com/source/drivers/net/ethernet/aurora/nb8800.c
>> http://lxr.free-electrons.com/source/drivers/net/phy/at803x.c
>>
>> The relevant device tree nodes are:
>>
>> eth0: ethernet@26000 {
>> compatible = "sigma,smp8734-ethernet";
>> reg = <0x26000 0x800>;
>> interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&clkgen SYS_CLK>;
>> };
>>
>> ð0 {
>> phy-connection-type = "rgmii";
>> phy-handle = <ð0_phy>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> /* Atheros AR8035 */
>> eth0_phy: ethernet-phy@4 {
>> compatible = "ethernet-phy-id004d.d072",
>> "ethernet-phy-ieee802.3-c22";
>> interrupts = <37 IRQ_TYPE_EDGE_RISING>;
>> reg = <4>;
>> };
>> };
>>
>> If I comment the PHY "interrupts" property, then the PHY framework
>> falls back to polling.
>>
>> Am I forgetting important information?
>
> In our system we attach the at8031 over SGMII to the gianfar (Freescale eTSEC) and
> to fibre optics transceivers, which operate in fixed speeds. Getting this setup to
> work reliably was challenging for various reasons, maybe worth to note
>
> 1) fixed SGMII speed not working: link is up on both ends, but no data is passed
> 2) known issue with SGMII link not completing autonegotiation correctly, see [1]
> 3) once autoneg is started or chip is reset, MII_CTRL1000 can not be written to
> until autoneg is completed => breaks phy state machine when the driver loads with
> unplugged cable and tries to set fixed speed
>
> Unless you are using fixed speed links in your setup, none of those should affect
> it. My experience with at8031 attached to RGMII is that it is genphy compliant,
> therefore I would a) disable interrupts, and b) prevent loading at803x.ko and try
> the genphy instead. Yours is an at8035, results may vary.
>
> [1] https://www.spinics.net/lists/netdev/msg400804.html
Hello Zefir,
Thanks for having a look at my issue.
I must confess that I'm not up to speed on the different *MIIs
(MII, RMII, GMII, RGMII, SGMII, etc)
One thing I find confusing... If the 8035 could work with the generic
PHY driver, what's the point of the at803x.ko driver?
Regards.
^ permalink raw reply
* Re: [PATCH v2 net-next] phy: increase size of bus_id and MII_BUS_ID_SIZE
From: Andrew Lunn @ 2017-01-19 14:25 UTC (permalink / raw)
To: Volodymyr Bendiuga
Cc: f.fainelli, netdev, volodymyr.bendiuga, Magnus Öberg
In-Reply-To: <1484826415-31631-1-git-send-email-volodymyr.bendiuga@gmail.com>
On Thu, Jan 19, 2017 at 12:46:55PM +0100, Volodymyr Bendiuga wrote:
> Some bus names are pretty long and do not fit into
> 20 chars, therefore bus_id size is increased together
> with MII_BUS_ID_SIZE to host larger names.
>
> Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@gmail.com>
> Signed-off-by: Magnus Öberg <magnus.oberg@westermo.se>
> ---
> include/linux/phy.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index f7d95f6..f53f0cd 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -162,7 +162,7 @@ static inline const char *phy_modes(phy_interface_t interface)
> * Need to be a little smaller than phydev->dev.bus_id to leave room
> * for the ":%02x"
It would also be good to fixup this comment. dev.bus_id does not seem
to exist any more. I think it got replaced by dev->kobj.name, which is
now a char * rather than an array. I think the comment can be removed.
> */
> -#define MII_BUS_ID_SIZE (20 - 3)
> +#define MII_BUS_ID_SIZE (64 - 3)
>
> /* Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the 21 bit
> IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy chips. */
> @@ -632,7 +632,7 @@ struct phy_driver {
> /* A Structure for boards to register fixups with the PHY Lib */
> struct phy_fixup {
> struct list_head list;
> - char bus_id[20];
> + char bus_id[64];
[MII_BUS_ID_SIZE + 3] would be better.
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
From: Alexander Duyck @ 2017-01-19 15:54 UTC (permalink / raw)
To: David Laight
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0267C5A@AcuExch.aculab.com>
On Thu, Jan 19, 2017 at 3:43 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Alexander Duyck
>> Sent: 18 January 2017 17:25
>> On Wed, Jan 18, 2017 at 8:22 AM, David Laight <David.Laight@aculab.com> wrote:
>> > From: David Miller
>> >> Sent: 17 January 2017 19:16
>> >> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
>> >> > enhance the performance for some cpu architecure, such as SPARC and so on.
>> >> > Currently it only supports one special cpu architecture(SPARC) in 82599
>> >> > driver to enable RO feature, this is not very common for other cpu architecture
>> >> > which really needs RO feature.
>> >> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
>> >> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>> >> >
>> >> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> >>
>> >> Since no-one has reviewed this patch, and I do not feel comfortable with applying
>> >> it without such review, I am tossing this patch.
>> >>
>> >> If someone eventually reviews it, repost this patch.
>> >
>> > Having re-read parts of the PCIe spec I think I'd like someone to
>> > explain exactly which transfers are affected by the 'relaxed ordering'
>> > bit and why any re-ordered transactions aren't a problem.
>> >
>> > In particular I believe RO allows the write to update the receive
>> > descriptor ring to overtake a write of receive packet data.
>> > That could lead to the network stack processing a receive frame
>> > before it has actually been written.
>> >
>> > David
>> >
>>
>> The Relaxed Ordering attribute doesn't get applied across the board.
>> It ends up being limited to a subset of the transactions if I recall
>> correctly. In this case it is the Tx descriptor write back, and the
>> Rx data write back. We don't apply the RO bit to any other
>> transactions.
>>
>> In the case of Tx descriptor there is no harm in allowing it to be
>> reordered because we only really read the DD bit so we don't care
>> about the ordering of the write back. In the case of the Rx data the
>> Rx descriptor essentially acts as a flush since it is sent without the
>> RO bit set. So all the writes before it must be completed before the
>> Rx descriptor write back.
>
> In which case why not set it unconditionally for all architectures?
>
> I'm surprised (I often am) that allowing those re-orderings makes
> any significant difference.
> Unfortunately you need a PCIe analyser to see what is really happening
> and they don't come cheap.
>
> What I do vaguely remember is that some hosts don't always implement
> the 'normal' re-ordering of reads and read completions.
> Re-ordering of reads allows descriptor reads to overtake transmit
> traffic which is likely to make a difference.
I think part of the issue, at least in the case of SPARC, is that the
handling of the memory writes in the PCIe root complex is impacted by
the RO attribute. On the bus itself it doesn't matter much, but at
the root complex it can become expensive to have to wait on a partial
write to complete while there are other writes pending. This is why
the IOMMU for SPARC now has a WEAK_ORDERING attribute you can add so
that it can write the data in whatever order it wants in relation to
other writes in that region.
- Alex
^ permalink raw reply
* [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM
From: Andrei Pistirica @ 2017-01-19 15:56 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam
Cc: boris.brezillon, rafalo, alexandre.belloni, michals,
Andrei Pistirica, tbultel, anirudh, punnaia, richardcochran
This patch does the following:
- MACB/GEM-PTP interface
- registers and bitfields for TSU
- capability flags to enable PTP per platform basis
Signed-off-by: Andrei Pistirica <andrei.pistirica@microchip.com>
---
Patch history:
Version 1:
This is just the common code for MACB/GEM-PTP support.
Code is based on the comments related to the following patch series:
- [RFC PATCH net-next v1-to-4 1/2] macb: Add 1588 support in Cadence GEM
- [RFC PATCH net-next v1-to-4 2/2] macb: Enable 1588 support in SAMA5Dx platforms
Version 2:
- Cosmetic changes and PTP capability flag changed doe to overlapping with JUMBO.
Note: Patch on net-next: January 19.
drivers/net/ethernet/cadence/macb.c | 32 +++++++++++++++-
drivers/net/ethernet/cadence/macb.h | 74 +++++++++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index c0fb80a..ff1e648 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2085,6 +2085,9 @@ static int macb_open(struct net_device *dev)
netif_tx_start_all_queues(dev);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_init(dev);
+
return 0;
}
@@ -2106,6 +2109,9 @@ static int macb_close(struct net_device *dev)
macb_free_consistent(bp);
+ if (bp->ptp_info)
+ bp->ptp_info->ptp_remove(dev);
+
return 0;
}
@@ -2379,6 +2385,17 @@ static int macb_set_ringparam(struct net_device *netdev,
return 0;
}
+static int macb_get_ts_info(struct net_device *netdev,
+ struct ethtool_ts_info *info)
+{
+ struct macb *bp = netdev_priv(netdev);
+
+ if (bp->ptp_info)
+ return bp->ptp_info->get_ts_info(netdev, info);
+
+ return ethtool_op_get_ts_info(netdev, info);
+}
+
static const struct ethtool_ops macb_ethtool_ops = {
.get_regs_len = macb_get_regs_len,
.get_regs = macb_get_regs,
@@ -2396,7 +2413,7 @@ static const struct ethtool_ops gem_ethtool_ops = {
.get_regs_len = macb_get_regs_len,
.get_regs = macb_get_regs,
.get_link = ethtool_op_get_link,
- .get_ts_info = ethtool_op_get_ts_info,
+ .get_ts_info = macb_get_ts_info,
.get_ethtool_stats = gem_get_ethtool_stats,
.get_strings = gem_get_ethtool_strings,
.get_sset_count = gem_get_sset_count,
@@ -2409,6 +2426,7 @@ static const struct ethtool_ops gem_ethtool_ops = {
static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
struct phy_device *phydev = dev->phydev;
+ struct macb *bp = netdev_priv(dev);
if (!netif_running(dev))
return -EINVAL;
@@ -2416,7 +2434,17 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
if (!phydev)
return -ENODEV;
- return phy_mii_ioctl(phydev, rq, cmd);
+ if (!bp->ptp_info)
+ return phy_mii_ioctl(phydev, rq, cmd);
+
+ switch (cmd) {
+ case SIOCSHWTSTAMP:
+ return bp->ptp_info->set_hwtst(dev, rq, cmd);
+ case SIOCGHWTSTAMP:
+ return bp->ptp_info->get_hwtst(dev, rq);
+ default:
+ return phy_mii_ioctl(phydev, rq, cmd);
+ }
}
static int macb_set_features(struct net_device *netdev,
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d67adad..94ddedd 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -131,6 +131,20 @@
#define GEM_RXIPCCNT 0x01a8 /* IP header Checksum Error Counter */
#define GEM_RXTCPCCNT 0x01ac /* TCP Checksum Error Counter */
#define GEM_RXUDPCCNT 0x01b0 /* UDP Checksum Error Counter */
+#define GEM_TISUBN 0x01bc /* 1588 Timer Increment Sub-ns */
+#define GEM_TSH 0x01c0 /* 1588 Timer Seconds High */
+#define GEM_TSL 0x01d0 /* 1588 Timer Seconds Low */
+#define GEM_TN 0x01d4 /* 1588 Timer Nanoseconds */
+#define GEM_TA 0x01d8 /* 1588 Timer Adjust */
+#define GEM_TI 0x01dc /* 1588 Timer Increment */
+#define GEM_EFTSL 0x01e0 /* PTP Event Frame Tx Seconds Low */
+#define GEM_EFTN 0x01e4 /* PTP Event Frame Tx Nanoseconds */
+#define GEM_EFRSL 0x01e8 /* PTP Event Frame Rx Seconds Low */
+#define GEM_EFRN 0x01ec /* PTP Event Frame Rx Nanoseconds */
+#define GEM_PEFTSL 0x01f0 /* PTP Peer Event Frame Tx Secs Low */
+#define GEM_PEFTN 0x01f4 /* PTP Peer Event Frame Tx Ns */
+#define GEM_PEFRSL 0x01f8 /* PTP Peer Event Frame Rx Sec Low */
+#define GEM_PEFRN 0x01fc /* PTP Peer Event Frame Rx Ns */
#define GEM_DCFG1 0x0280 /* Design Config 1 */
#define GEM_DCFG2 0x0284 /* Design Config 2 */
#define GEM_DCFG3 0x0288 /* Design Config 3 */
@@ -174,6 +188,7 @@
#define MACB_NCR_TPF_SIZE 1
#define MACB_TZQ_OFFSET 12 /* Transmit zero quantum pause frame */
#define MACB_TZQ_SIZE 1
+#define MACB_SRTSM_OFFSET 15
/* Bitfields in NCFGR */
#define MACB_SPD_OFFSET 0 /* Speed */
@@ -319,6 +334,32 @@
#define MACB_PTZ_SIZE 1
#define MACB_WOL_OFFSET 14 /* Enable wake-on-lan interrupt */
#define MACB_WOL_SIZE 1
+#define MACB_DRQFR_OFFSET 18 /* PTP Delay Request Frame Received */
+#define MACB_DRQFR_SIZE 1
+#define MACB_SFR_OFFSET 19 /* PTP Sync Frame Received */
+#define MACB_SFR_SIZE 1
+#define MACB_DRQFT_OFFSET 20 /* PTP Delay Request Frame Transmitted */
+#define MACB_DRQFT_SIZE 1
+#define MACB_SFT_OFFSET 21 /* PTP Sync Frame Transmitted */
+#define MACB_SFT_SIZE 1
+#define MACB_PDRQFR_OFFSET 22 /* PDelay Request Frame Received */
+#define MACB_PDRQFR_SIZE 1
+#define MACB_PDRSFR_OFFSET 23 /* PDelay Response Frame Received */
+#define MACB_PDRSFR_SIZE 1
+#define MACB_PDRQFT_OFFSET 24 /* PDelay Request Frame Transmitted */
+#define MACB_PDRQFT_SIZE 1
+#define MACB_PDRSFT_OFFSET 25 /* PDelay Response Frame Transmitted */
+#define MACB_PDRSFT_SIZE 1
+#define MACB_SRI_OFFSET 26 /* TSU Seconds Register Increment */
+#define MACB_SRI_SIZE 1
+
+/* Timer increment fields */
+#define MACB_TI_CNS_OFFSET 0
+#define MACB_TI_CNS_SIZE 8
+#define MACB_TI_ACNS_OFFSET 8
+#define MACB_TI_ACNS_SIZE 8
+#define MACB_TI_NIT_OFFSET 16
+#define MACB_TI_NIT_SIZE 8
/* Bitfields in MAN */
#define MACB_DATA_OFFSET 0 /* data */
@@ -386,6 +427,17 @@
#define GEM_PBUF_LSO_OFFSET 27
#define GEM_PBUF_LSO_SIZE 1
+/* Bitfields in TISUBN */
+#define GEM_SUBNSINCR_OFFSET 0
+#define GEM_SUBNSINCR_SIZE 16
+
+/* Bitfields in TI */
+#define GEM_NSINCR_OFFSET 0
+#define GEM_NSINCR_SIZE 8
+
+/* Bitfields in ADJ */
+#define GEM_ADDSUB_OFFSET 31
+#define GEM_ADDSUB_SIZE 1
/* Constants for CLK */
#define MACB_CLK_DIV8 0
#define MACB_CLK_DIV16 1
@@ -413,6 +465,7 @@
#define MACB_CAPS_NO_GIGABIT_HALF 0x00000008
#define MACB_CAPS_USRIO_DISABLED 0x00000010
#define MACB_CAPS_JUMBO 0x00000020
+#define MACB_CAPS_GEM_HAS_PTP 0x00000040
#define MACB_CAPS_FIFO_MODE 0x10000000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
@@ -782,6 +835,20 @@ struct macb_or_gem_ops {
int (*mog_rx)(struct macb *bp, int budget);
};
+/* MACB-PTP interface: adapt to platform needs. */
+struct macb_ptp_info {
+ void (*ptp_init)(struct net_device *ndev);
+ void (*ptp_remove)(struct net_device *ndev);
+ s32 (*get_ptp_max_adj)(void);
+ unsigned int (*get_tsu_rate)(struct macb *bp);
+ int (*get_ts_info)(struct net_device *dev,
+ struct ethtool_ts_info *info);
+ int (*get_hwtst)(struct net_device *netdev,
+ struct ifreq *ifr);
+ int (*set_hwtst)(struct net_device *netdev,
+ struct ifreq *ifr, int cmd);
+};
+
struct macb_config {
u32 caps;
unsigned int dma_burst_length;
@@ -874,6 +941,8 @@ struct macb {
unsigned int jumbo_max_len;
u32 wol;
+
+ struct macb_ptp_info *ptp_info; /* macb-ptp interface */
};
static inline bool macb_is_gem(struct macb *bp)
@@ -881,4 +950,9 @@ static inline bool macb_is_gem(struct macb *bp)
return !!(bp->caps & MACB_CAPS_MACB_IS_GEM);
}
+static inline bool gem_has_ptp(struct macb *bp)
+{
+ return !!(bp->caps & MACB_CAPS_GEM_HAS_PTP);
+}
+
#endif /* _MACB_H */
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] MAINTAINERS: update cxgb4 maintainer
From: David Miller @ 2017-01-19 15:57 UTC (permalink / raw)
To: hariprasad; +Cc: netdev, nirranjan, ganeshgr
In-Reply-To: <1484806832-15756-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Thu, 19 Jan 2017 11:50:32 +0530
> Ganesg will be taking over as maintainer from now
>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ethtool: avoid allocation failure for dump_regs
From: John W. Linville @ 2017-01-19 15:56 UTC (permalink / raw)
To: David Arcari; +Cc: David Miller, netdev, Johannes Berg, Kalle Valo
In-Reply-To: <20170119141508.GA6245@tuxdriver.com>
I forgot to Cc Johannes and Kalle...
On Thu, Jan 19, 2017 at 09:15:09AM -0500, John W. Linville wrote:
> On Thu, Jan 19, 2017 at 07:35:22AM -0500, David Arcari wrote:
> > On 01/18/2017 11:45 AM, David Miller wrote:
> > > From: David Arcari <darcari@redhat.com>
> > > Date: Wed, 18 Jan 2017 08:34:05 -0500
> > >
> > >> If the user executes 'ethtool -d' for an interface and the associated
> > >> get_regs_len() function returns 0, the user will see a call trace from
> > >> the vmalloc() call in ethtool_get_regs(). This patch modifies
> > >> ethtool_get_regs() to avoid the call to vmalloc when the size is zero.
> > >>
> > >> Signed-off-by: David Arcari <darcari@redhat.com>
> > > I think when the driver indicates this, it is equivalent to saying that
> > > the operation isn't supported.
> > >
> > > Also, this guards us against ->get_regs() methods that don't handle
> > > zero length requests properly. I see many which are going to do
> > > really terrible things in that situation.
> > >
> > > Therefore, if get_regs_len() returns zero, treat it the safe as if the
> > > ethtool operations were NULL.
> > >
> > > Thanks.
> >
> > That was actually the fix that I was originally considering, but it
> > turns out
> > there is a problem with it.
> >
> > I found that the vmalloc error was occurring because
> > ieee80211_get_regs_len() in
> > net/mac80211/ethtool.c was returning zero. The ieee80211_get_regs in
> > the same
> > file returns the hw version. It turns out that this information is used
> > by the
> > at76c50x-usb driver in the user space ethtool to report which HW variant
> > is in
> > use. Returning an error when regs_len() returns zero would break this
> > functionality.
> >
> > -Dave
>
> I'm responsible for this mess. The original idea was for various
> mac80211-based drivers to override the ethtool operation and provide
> their own dump operation, but the mac80211 crowd never embraced
> the idea.
>
> In the meantime, I added the default implementation which just
> passed-up wdev->wiphy->hw_version as the version info for a 0-length
> register dump. I then implemented a driver-specific regiser dump
> handler for userland ethtool that would interpret the hardware version
> information for the at76c50x-usb driver.
>
> So the net of it is, if we treat a return of 0 from get_regs_len()
> as "not supported", we break this one driver-specific feature for
> userland ethtool. Realistically, there are probably very few users
> to care. But I can't guarantee that the number is zero.
>
> Possible solutions:
>
> -- break userland ethtool for at76c50x-usb
> -- avoid 0-len allocation attempt (David Arcari's patch)
> -- make allocator accept a 0 length value w/o oops'ing
> -- change mac8011 code to return non-zero from get_regs_len()
>
> Thoughts? The last option holds a certain attraction, but I'm not
> sure how to make it useful...?
>
> John
>
> --
> John W. Linville Someday the world will need a hero, and you
> linville@tuxdriver.com might be all we have. Be ready.
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ 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