* [RFC, iproute2 PATCH v2] tc/mqprio: Offload mode and shaper options in mqprio
From: Amritha Nambiar @ 2017-09-07 11:03 UTC (permalink / raw)
To: stephen, netdev; +Cc: alexander.h.duyck, amritha.nambiar
Adds new mqprio options for 'mode' and 'shaper'. The mode
option can take values for offload modes such as 'dcb' (default),
'channel' with the 'hw' option set to 1. The 'shaper' option is
to support HW shapers ('dcb' default) and takes the value
'bw_rlimit' for bandwidth rate limiting. The parameters to the
bw_rlimit shaper are minimum and maximum bandwidth rates.
New HW shapers in future can be supported through the shaper
attribute.
# tc qdisc add dev eth0 root mqprio num_tc 2 map 0 0 0 0 1 1 1 1\
queues 4@0 4@4 hw 1 mode channel shaper bw_rlimit\
min_rate 1Gbit 2Gbit max_rate 4Gbit 5Gbit
# tc qdisc show dev eth0
qdisc mqprio 804a: root tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
queues:(0:3) (4:7)
mode:channel
shaper:bw_rlimit min_rate:1Gbit 2Gbit max_rate:4Gbit 5Gbit
Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
include/linux/pkt_sched.h | 32 ++++++++
tc/q_mqprio.c | 191 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 216 insertions(+), 7 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 099bf55..e95b5c9 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -625,6 +625,22 @@ enum {
#define TC_MQPRIO_HW_OFFLOAD_MAX (__TC_MQPRIO_HW_OFFLOAD_MAX - 1)
+enum {
+ TC_MQPRIO_MODE_DCB,
+ TC_MQPRIO_MODE_CHANNEL,
+ __TC_MQPRIO_MODE_MAX
+};
+
+#define __TC_MQPRIO_MODE_MAX (__TC_MQPRIO_MODE_MAX - 1)
+
+enum {
+ TC_MQPRIO_SHAPER_DCB,
+ TC_MQPRIO_SHAPER_BW_RATE, /* Add new shapers below */
+ __TC_MQPRIO_SHAPER_MAX
+};
+
+#define __TC_MQPRIO_SHAPER_MAX (__TC_MQPRIO_SHAPER_MAX - 1)
+
struct tc_mqprio_qopt {
__u8 num_tc;
__u8 prio_tc_map[TC_QOPT_BITMASK + 1];
@@ -633,6 +649,22 @@ struct tc_mqprio_qopt {
__u16 offset[TC_QOPT_MAX_QUEUE];
};
+#define TC_MQPRIO_F_MODE 0x1
+#define TC_MQPRIO_F_SHAPER 0x2
+#define TC_MQPRIO_F_MIN_RATE 0x4
+#define TC_MQPRIO_F_MAX_RATE 0x8
+
+enum {
+ TCA_MQPRIO_UNSPEC,
+ TCA_MQPRIO_MODE,
+ TCA_MQPRIO_SHAPER,
+ TCA_MQPRIO_MIN_RATE64,
+ TCA_MQPRIO_MAX_RATE64,
+ __TCA_MQPRIO_MAX,
+};
+
+#define TCA_MQPRIO_MAX (__TCA_MQPRIO_MAX - 1)
+
/* SFB */
enum {
diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
index d6718fb..5fec63d 100644
--- a/tc/q_mqprio.c
+++ b/tc/q_mqprio.c
@@ -27,6 +27,10 @@ static void explain(void)
fprintf(stderr, "Usage: ... mqprio [num_tc NUMBER] [map P0 P1 ...]\n");
fprintf(stderr, " [queues count1@offset1 count2@offset2 ...] ");
fprintf(stderr, "[hw 1|0]\n");
+ fprintf(stderr, " [mode dcb|channel]\n");
+ fprintf(stderr, " [shaper bw_rlimit SHAPER_PARAMS]\n"
+ "Where: SHAPER_PARAMS := { min_rate MIN_RATE1 MIN_RATE2 ...|\n"
+ " max_rate MAX_RATE1 MAX_RATE2 ... }\n");
}
static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
@@ -40,6 +44,12 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
.count = { },
.offset = { },
};
+ __u64 min_rate64[TC_QOPT_MAX_QUEUE] = {0};
+ __u64 max_rate64[TC_QOPT_MAX_QUEUE] = {0};
+ __u16 shaper = TC_MQPRIO_SHAPER_DCB;
+ __u16 mode = TC_MQPRIO_MODE_DCB;
+ struct rtattr *tail;
+ __u32 flags = 0;
while (argc > 0) {
idx = 0;
@@ -92,6 +102,68 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
return -1;
}
idx++;
+ } else if (opt.hw && strcmp(*argv, "mode") == 0) {
+ NEXT_ARG();
+ if (matches(*argv, "dcb") == 0) {
+ mode = TC_MQPRIO_MODE_DCB;
+ } else if (matches(*argv, "channel") == 0) {
+ mode = TC_MQPRIO_MODE_CHANNEL;
+ } else {
+ fprintf(stderr, "Illegal mode (%s)\n",
+ *argv);
+ return -1;
+ }
+ if (mode != TC_MQPRIO_MODE_DCB)
+ flags |= TC_MQPRIO_F_MODE;
+ idx++;
+ } else if (opt.hw && strcmp(*argv, "shaper") == 0) {
+ NEXT_ARG();
+ if (matches(*argv, "dcb") == 0) {
+ shaper = TC_MQPRIO_SHAPER_DCB;
+ } else if (matches(*argv, "bw_rlimit") == 0) {
+ shaper = TC_MQPRIO_SHAPER_BW_RATE;
+ if (!NEXT_ARG_OK()) {
+ fprintf(stderr, "Incomplete shaper arguments\n");
+ return -1;
+ }
+ } else {
+ fprintf(stderr, "Illegal shaper (%s)\n",
+ *argv);
+ return -1;
+ }
+ if (shaper != TC_MQPRIO_SHAPER_DCB)
+ flags |= TC_MQPRIO_F_SHAPER;
+ idx++;
+ } else if ((shaper == TC_MQPRIO_SHAPER_BW_RATE) &&
+ strcmp(*argv, "min_rate") == 0) {
+ while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+ NEXT_ARG();
+ if (get_rate64(&min_rate64[idx], *argv)) {
+ PREV_ARG();
+ break;
+ }
+ idx++;
+ }
+ if ((idx < opt.num_tc) && !NEXT_ARG_OK()) {
+ fprintf(stderr, "Incomplete arguments, min_rate values expected\n");
+ return -1;
+ }
+ flags |= TC_MQPRIO_F_MIN_RATE;
+ } else if ((shaper == TC_MQPRIO_SHAPER_BW_RATE) &&
+ strcmp(*argv, "max_rate") == 0) {
+ while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+ NEXT_ARG();
+ if (get_rate64(&max_rate64[idx], *argv)) {
+ PREV_ARG();
+ break;
+ }
+ idx++;
+ }
+ if ((idx < opt.num_tc) && !NEXT_ARG_OK()) {
+ fprintf(stderr, "Incomplete arguments, max_rate values expected\n");
+ return -1;
+ }
+ flags |= TC_MQPRIO_F_MAX_RATE;
} else if (strcmp(*argv, "help") == 0) {
explain();
return -1;
@@ -102,27 +174,132 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
argc--; argv++;
}
+ tail = NLMSG_TAIL(n);
addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+
+ if (flags & TC_MQPRIO_F_MODE)
+ addattr_l(n, 1024, TCA_MQPRIO_MODE,
+ &mode, sizeof(mode));
+ if (flags & TC_MQPRIO_F_SHAPER)
+ addattr_l(n, 1024, TCA_MQPRIO_SHAPER,
+ &shaper, sizeof(shaper));
+
+ if (flags & TC_MQPRIO_F_MIN_RATE) {
+ struct rtattr *start;
+
+ start = addattr_nest(n, 1024,
+ TCA_MQPRIO_MIN_RATE64 | NLA_F_NESTED);
+
+ for (idx = 0; idx < TC_QOPT_MAX_QUEUE; idx++)
+ addattr_l(n, 1024, TCA_MQPRIO_MIN_RATE64,
+ &min_rate64[idx], sizeof(min_rate64[idx]));
+
+ addattr_nest_end(n, start);
+ }
+
+ if (flags & TC_MQPRIO_F_MAX_RATE) {
+ struct rtattr *start;
+
+ start = addattr_nest(n, 1024,
+ TCA_MQPRIO_MAX_RATE64 | NLA_F_NESTED);
+
+ for (idx = 0; idx < TC_QOPT_MAX_QUEUE; idx++)
+ addattr_l(n, 1024, TCA_MQPRIO_MAX_RATE64,
+ &max_rate64[idx], sizeof(max_rate64[idx]));
+
+ addattr_nest_end(n, start);
+ }
+
+ tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+
return 0;
}
static int mqprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
{
int i;
- struct tc_mqprio_qopt *qopt;
+ struct tc_mqprio_qopt qopt;
+ __u64 min_rate64[TC_QOPT_MAX_QUEUE] = {0};
+ __u64 max_rate64[TC_QOPT_MAX_QUEUE] = {0};
+ int len = RTA_PAYLOAD(opt) - RTA_ALIGN(sizeof(qopt));
+
+ SPRINT_BUF(b1);
if (opt == NULL)
return 0;
- qopt = RTA_DATA(opt);
+ if (len < 0) {
+ fprintf(stderr, "options size error\n");
+ return -1;
+ }
+
+ memcpy(&qopt, RTA_DATA(opt), RTA_ALIGN(sizeof(qopt)));
- fprintf(f, " tc %u map ", qopt->num_tc);
+ fprintf(f, " tc %u map ", qopt.num_tc);
for (i = 0; i <= TC_PRIO_MAX; i++)
- fprintf(f, "%u ", qopt->prio_tc_map[i]);
+ fprintf(f, "%u ", qopt.prio_tc_map[i]);
fprintf(f, "\n queues:");
- for (i = 0; i < qopt->num_tc; i++)
- fprintf(f, "(%u:%u) ", qopt->offset[i],
- qopt->offset[i] + qopt->count[i] - 1);
+ for (i = 0; i < qopt.num_tc; i++)
+ fprintf(f, "(%u:%u) ", qopt.offset[i],
+ qopt.offset[i] + qopt.count[i] - 1);
+
+ if (len > 0) {
+ struct rtattr *tb[TCA_MQPRIO_MAX + 1];
+
+ parse_rtattr(tb, TCA_MQPRIO_MAX,
+ RTA_DATA(opt) + RTA_ALIGN(sizeof(qopt)),
+ len);
+
+ if (tb[TCA_MQPRIO_MODE]) {
+ __u16 *mode = RTA_DATA(tb[TCA_MQPRIO_MODE]);
+
+ if (*mode == TC_MQPRIO_MODE_CHANNEL)
+ fprintf(f, "\n mode:channel");
+ } else {
+ fprintf(f, "\n mode:dcb");
+ }
+
+ if (tb[TCA_MQPRIO_SHAPER]) {
+ __u16 *shaper = RTA_DATA(tb[TCA_MQPRIO_SHAPER]);
+
+ if (*shaper == TC_MQPRIO_SHAPER_BW_RATE)
+ fprintf(f, "\n shaper:bw_rlimit");
+ } else {
+ fprintf(f, "\n shaper:dcb");
+ }
+
+ if (tb[TCA_MQPRIO_MIN_RATE64]) {
+ struct rtattr *r;
+ int rem = RTA_PAYLOAD(tb[TCA_MQPRIO_MIN_RATE64]);
+ __u64 *min = min_rate64;
+
+ for (r = RTA_DATA(tb[TCA_MQPRIO_MIN_RATE64]);
+ RTA_OK(r, rem); r = RTA_NEXT(r, rem)) {
+ if (r->rta_type != TCA_MQPRIO_MIN_RATE64)
+ return -1;
+ *(min++) = rta_getattr_u64(r);
+ }
+ fprintf(f, " min_rate:");
+ for (i = 0; i < qopt.num_tc; i++)
+ fprintf(f, "%s ", sprint_rate(min_rate64[i], b1));
+ }
+
+ if (tb[TCA_MQPRIO_MAX_RATE64]) {
+ struct rtattr *r;
+ int rem = RTA_PAYLOAD(tb[TCA_MQPRIO_MAX_RATE64]);
+ __u64 *max = max_rate64;
+
+ for (r = RTA_DATA(tb[TCA_MQPRIO_MAX_RATE64]);
+ RTA_OK(r, rem); r = RTA_NEXT(r, rem)) {
+ if (r->rta_type != TCA_MQPRIO_MAX_RATE64)
+ return -1;
+ *(max++) = rta_getattr_u64(r);
+ }
+ fprintf(f, " max_rate:");
+ for (i = 0; i < qopt.num_tc; i++)
+ fprintf(f, "%s ", sprint_rate(max_rate64[i], b1));
+ }
+ }
return 0;
}
^ permalink raw reply related
* Re: [PATCH] netfilter: xt_hashlimit: avoid 64-bit division
From: Arnd Bergmann @ 2017-09-07 11:16 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Vishwanath Pai, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Josh Hunt, netfilter-devel, coreteam, Networking,
Linux Kernel Mailing List
In-Reply-To: <20170907101931.GB2049@salvia>
On Thu, Sep 7, 2017 at 12:19 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Sep 06, 2017 at 10:48:22PM +0200, Arnd Bergmann wrote:
>> On Wed, Sep 6, 2017 at 10:22 PM, Vishwanath Pai <vpai@akamai.com> wrote:
>> > On 09/06/2017 03:57 PM, Arnd Bergmann wrote:
>> >> 64-bit division is expensive on 32-bit architectures, and
>> >> requires a special function call to avoid a link error like:
>> >>
>> >> net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
>> >> xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'
>> >>
>> >> In the case of hashlimit_mt_common, we don't actually need a
>> >> 64-bit operation, we can simply rewrite the function slightly
>> >> to make that clear to the compiler.
>> >>
>> >> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
>> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> >> ---
>> >> net/netfilter/xt_hashlimit.c | 5 ++++-
>> >> 1 file changed, 4 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
>> >> index 10d48234f5f4..50b53d86eef5 100644
>> >> --- a/net/netfilter/xt_hashlimit.c
>> >> +++ b/net/netfilter/xt_hashlimit.c
>> >> @@ -531,7 +531,10 @@ static u64 user2rate_bytes(u64 user)
>> >> {
>> >> u64 r;
>> >>
>> >> - r = user ? 0xFFFFFFFFULL / user : 0xFFFFFFFFULL;
>> >> + if (user > 0xFFFFFFFFULL)
>> >> + return 0;
>> >> +
>> >> + r = user ? 0xFFFFFFFFULL / (u32)user : 0xFFFFFFFFULL;
>> >> r = (r - 1) << 4;
>> >> return r;
>> >> }
>> >>
>> >
>> > I have submitted another patch to fix this:
>> > https://patchwork.ozlabs.org/patch/809881/
>> >
>> > We have seen this problem before, I was careful not to introduce this
>> > again in the new patch but clearly I overlooked this particular line :(
>> >
>> > In the other cases we fixed it by replacing division with div64_u64().
>>
>> div64_u64() seems needlessly expensive here since the dividend
>> is known to be a 32-bit number. I guess the function is not called
>> frequently though, so it doesn't matter much.
>
> This is called from the packet path, only for the first packet for
> each new destination IP entry in the hashtable, still from the
> datapath. So if we can take something faster (for 32 bit arches) that
> is correct, I think it's sensible to take.
>
> Let me know in any case.
I think my version should be slightly better then, unless someone
finds something wrong with it.
Arnd
^ permalink raw reply
* [PATCH] drivers: net: Display proper debug level up to 6
From: Mathieu Malaterre @ 2017-09-07 11:24 UTC (permalink / raw)
Cc: Mathieu Malaterre, David S. Miller, Philippe Reynes,
Johannes Berg, Jarod Wilson, netdev, linux-kernel
This will make it explicit some messages are of the form:
dm9000_dbg(db, 5, ...
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
drivers/net/ethernet/davicom/dm9000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c
index 16fe776ddbe5..50222b7b81f3 100644
--- a/drivers/net/ethernet/davicom/dm9000.c
+++ b/drivers/net/ethernet/davicom/dm9000.c
@@ -65,7 +65,7 @@ MODULE_PARM_DESC(watchdog, "transmit timeout in milliseconds");
*/
static int debug;
module_param(debug, int, 0644);
-MODULE_PARM_DESC(debug, "dm9000 debug level (0-4)");
+MODULE_PARM_DESC(debug, "dm9000 debug level (0-6)");
/* DM9000 register address locking.
*
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] ath10: mark PM functions as __maybe_unused
From: Kalle Valo @ 2017-09-07 11:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Colin Ian King, bartosz.markowski@tieto.com, Govind Singh,
Ryan Hsu, Srinivas Kandagatla, Rajkumar Manoharan,
Ashok Raj Nagarajan, Ben Greear, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170906125904.2588620-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> When CONFIG_PM_SLEEP is disabled, we get a compile-time
> warning:
>
> drivers/net/wireless/ath/ath10k/pci.c:3417:12: error: 'ath10k_pci_pm_resume' defined but not used [-Werror=unused-function]
> static int ath10k_pci_pm_resume(struct device *dev)
> ^~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/pci.c:3401:12: error: 'ath10k_pci_pm_suspend' defined but not used [-Werror=unused-function]
> static int ath10k_pci_pm_suspend(struct device *dev)
>
> Rather than fixing the #ifdef, this just marks both functions
> as __maybe_unused, which is a more robust way to do this.
>
> Fixes: 32faa3f0ee50 ("ath10k: add the PCI PM core suspend/resume ops")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks. But the title should have "ath10k:", I'll fix that.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module
From: Stephen Smalley @ 2017-09-07 12:32 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, Chenbo Feng, linux-security-module, SELinux,
Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <CAMOXUJ=r1AEO_6Pg7-tio9UPn2YCJKOrLS=xbn7U4SE_mBYyJQ@mail.gmail.com>
On Tue, 2017-09-05 at 15:24 -0700, Chenbo Feng via Selinux wrote:
> On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> > > From: Chenbo Feng <fengc@google.com>
> > >
> > > Introduce 5 LSM hooks to provide finer granularity controls on
> > > eBPF
> > > related operations including create eBPF maps, modify and read
> > > eBPF
> > > maps
> > > content and load eBPF programs to the kernel. Hooks use the new
> > > security
> > > pointer inside the eBPF map struct to store the owner's security
> > > information and the different security modules can perform
> > > different
> > > checks based on the information stored inside the security field.
> > >
> > > Signed-off-by: Chenbo Feng <fengc@google.com>
> > > ---
> > > include/linux/lsm_hooks.h | 41
> > > +++++++++++++++++++++++++++++++++++++++++
> > > include/linux/security.h | 36
> > > ++++++++++++++++++++++++++++++++++++
> > > security/security.c | 28 ++++++++++++++++++++++++++++
> > > 3 files changed, 105 insertions(+)
> > >
> > > diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h
> > > index ce02f76a6188..3aaf9a08a983 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1353,6 +1353,32 @@
> > > * @inode we wish to get the security context of.
> > > * @ctx is a pointer in which to place the allocated security
> > > context.
> > > * @ctxlen points to the place to put the length of @ctx.
> > > + *
> > > + * Security hooks for using the eBPF maps and programs
> > > functionalities through
> > > + * eBPF syscalls.
> > > + *
> > > + * @bpf_map_create:
> > > + * Check permissions prior to creating a new bpf map.
> > > + * Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_modify:
> > > + * Check permission prior to insert, update and delete map
> > > content.
> > > + * @map pointer to the struct bpf_map that contains map
> > > information.
> > > + * Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_read:
> > > + * Check permission prior to read a bpf map content.
> > > + * @map pointer to the struct bpf_map that contains map
> > > information.
> > > + * Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_prog_load:
> > > + * Check permission prior to load eBPF program.
> > > + * Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_post_create:
> > > + * Initialize the bpf object security field inside struct
> > > bpf_maps and
> > > + * it is used for future security checks.
> > > + *
> > > */
> > > union security_list_options {
> > > int (*binder_set_context_mgr)(struct task_struct *mgr);
> > > @@ -1685,6 +1711,14 @@ union security_list_options {
> > > struct audit_context *actx);
> > > void (*audit_rule_free)(void *lsmrule);
> > > #endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > + int (*bpf_map_create)(void);
> > > + int (*bpf_map_read)(struct bpf_map *map);
> > > + int (*bpf_map_modify)(struct bpf_map *map);
> > > + int (*bpf_prog_load)(void);
> > > + int (*bpf_post_create)(struct bpf_map *map);
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > > };
> > >
> > > struct security_hook_heads {
> > > @@ -1905,6 +1939,13 @@ struct security_hook_heads {
> > > struct list_head audit_rule_match;
> > > struct list_head audit_rule_free;
> > > #endif /* CONFIG_AUDIT */
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > + struct list_head bpf_map_create;
> > > + struct list_head bpf_map_read;
> > > + struct list_head bpf_map_modify;
> > > + struct list_head bpf_prog_load;
> > > + struct list_head bpf_post_create;
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > > } __randomize_layout;
> > >
> > > /*
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index 458e24bea2d4..0656a4f74d14 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -31,6 +31,7 @@
> > > #include <linux/string.h>
> > > #include <linux/mm.h>
> > > #include <linux/fs.h>
> > > +#include <linux/bpf.h>
> > >
> > > struct linux_binprm;
> > > struct cred;
> > > @@ -1735,6 +1736,41 @@ static inline void
> > > securityfs_remove(struct
> > > dentry *dentry)
> > >
> > > #endif
> > >
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +#ifdef CONFIG_SECURITY
> > > +int security_map_create(void);
> > > +int security_map_modify(struct bpf_map *map);
> > > +int security_map_read(struct bpf_map *map);
> > > +int security_prog_load(void);
> > > +int security_post_create(struct bpf_map *map);
> > > +#else
> > > +static inline int security_map_create(void)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int security_map_read(struct bpf_map *map)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int security_map_modify(struct bpf_map *map)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int security_prog_load(void)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int security_post_create(struct bpf_map *map)
> > > +{
> > > + return 0;
> > > +}
> > > +#endif /* CONFIG_SECURITY */
> > > +#endif /* CONFIG_BPF_SYSCALL */
> >
> > These should be named consistently with the ones in lsm_hooks.h and
> > should unambiguously indicate that these are hooks for bpf
> > objects/operations, i.e. security_bpf_map_create(),
> > security_bpf_map_read(), etc.
> >
>
> Thanks for pointing out, will fix this.
> > Do you need this level of granularity?
> >
>
> The cover letter of this patch series described a possible use cases
> of
> these lsm hooks and this level of granularity would be ideal to reach
> that
> goal. We can also implement two hooks such as bpf_obj_create and
> bpf_obj_use to restrict the creation and using when get the bpf fd
> from
> kernel. But that will be less powerful and flexible.
> > Could you coalesce the map_create() and post_map_create() hooks
> > into
> > one hook and just unwind the create in that case?
> >
>
> Okay, I will take a look on how to fix this.
Also, what you called security_post_create() would normally be called
something like security_bpf_alloc_security(), and would have a
corresponding security_bpf_free_security() hook too. However, whether
or not you still need this security field and hook at all is unclear to
me, given the direction the discussion has gone.
> > Why do you label bpf maps but not bpf progs? Should we be
> > controlling
> > the ability to attach/detach a bpf prog (partly controlled by
> > CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> > control based on who created the prog)?
> >
> > Should there be a top-level security_bpf_use() hook and permission
> > check that limits ability to use bpf() at all?
> >
>
> This could be useful but having additional lsm hooks check when
> reading
> and write to eBPF maps may cause performance issue. Instead maybe we
> could have a hook for creating eBPF object and retrieve object fd to
> restrict
> the access.
> > > +
> > > #ifdef CONFIG_SECURITY
> > >
> > > static inline char *alloc_secdata(void)
> > > diff --git a/security/security.c b/security/security.c
> > > index 55b5997e4b72..02272f93a89e 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -12,6 +12,7 @@
> > > * (at your option) any later version.
> > > */
> > >
> > > +#include <linux/bpf.h>
> > > #include <linux/capability.h>
> > > #include <linux/dcache.h>
> > > #include <linux/module.h>
> > > @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid,
> > > u32
> > > field, u32 op, void *lsmrule,
> > > actx);
> > > }
> > > #endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +int security_map_create(void)
> > > +{
> > > + return call_int_hook(bpf_map_create, 0);
> > > +}
> > > +
> > > +int security_map_modify(struct bpf_map *map)
> > > +{
> > > + return call_int_hook(bpf_map_modify, 0, map);
> > > +}
> > > +
> > > +int security_map_read(struct bpf_map *map)
> > > +{
> > > + return call_int_hook(bpf_map_read, 0, map);
> > > +}
> > > +
> > > +int security_prog_load(void)
> > > +{
> > > + return call_int_hook(bpf_prog_load, 0);
> > > +}
> > > +
> > > +int security_post_create(struct bpf_map *map)
> > > +{
> > > + return call_int_hook(bpf_post_create, 0, map);
> > > +}
> > > +#endif /* CONFIG_BPF_SYSCALL */
^ permalink raw reply
* [V2 PATCH net-next 0/2] Fixes for XDP_REDIRECT map
From: Jesper Dangaard Brouer @ 2017-09-07 12:33 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
Jesper Dangaard Brouer
This my V2 of catching XDP_REDIRECT and bpf_redirect_map() API usage
that can potentially crash the kernel. Addressed Daniels feedback in
patch01, and added patch02 which catch and cleanup dangling map
pointers.
I know John and Daniel are working on a more long-term solution, of
recording the bpf_prog pointer together with the map pointer. I just
wanted to propose these fixes as a stop-gap to the potential crashes.
---
Jesper Dangaard Brouer (2):
xdp: implement xdp_redirect_map for generic XDP
xdp: catch invalid XDP_REDIRECT API usage
include/linux/filter.h | 1 +
include/trace/events/xdp.h | 4 ++--
net/core/dev.c | 3 +++
net/core/filter.c | 39 ++++++++++++++++++++++++++++++++++++---
4 files changed, 42 insertions(+), 5 deletions(-)
^ permalink raw reply
* [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP
From: Jesper Dangaard Brouer @ 2017-09-07 12:33 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
Jesper Dangaard Brouer
In-Reply-To: <150478756604.28665.6915020425359475729.stgit@firesoul>
Using bpf_redirect_map is allowed for generic XDP programs, but the
appropriate map lookup was never performed in xdp_do_generic_redirect().
Instead the map-index is directly used as the ifindex. For the
xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
sending on ifindex 0 which isn't valid, resulting in getting SKB
packets dropped. Thus, the reported performance numbers are wrong in
commit 24251c264798 ("samples/bpf: add option for native and skb mode
for redirect apps") for the 'xdp_redirect_map -S' case.
It might seem innocent this was lacking, but it can actually crash the
kernel. The potential crash is caused by not consuming redirect_info->map.
The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
pointer, which will survive even after unloading the xdp bpf_prog and
deallocating the devmap data-structure. This leaves a dead map
pointer around. The kernel will crash when loading the xdp_redirect
sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
and returns XDP_REDIRECT, which will cause it to dereference the map
pointer.
Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
include/trace/events/xdp.h | 4 ++--
net/core/filter.c | 14 +++++++++++---
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 862575ac8da9..4e16c43fba10 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,
#define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0, \
- 0, map, idx);
+ 0, map, idx)
#define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \
trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0, \
- err, map, idx);
+ err, map, idx)
#endif /* _TRACE_XDP_H */
diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c738a7b2..3767470cab6c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
unsigned int len;
int err = 0;
- fwd = dev_get_by_index_rcu(dev_net(dev), index);
ri->ifindex = 0;
+ ri->map = NULL;
+
+ if (map)
+ fwd = __dev_map_lookup_elem(map, index);
+ else
+ fwd = dev_get_by_index_rcu(dev_net(dev), index);
if (unlikely(!fwd)) {
err = -EINVAL;
goto err;
@@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
}
skb->dev = fwd;
- _trace_xdp_redirect(dev, xdp_prog, index);
+ map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
+ : _trace_xdp_redirect(dev, xdp_prog, index);
return 0;
err:
- _trace_xdp_redirect_err(dev, xdp_prog, index, err);
+ map ? _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err)
+ : _trace_xdp_redirect_err(dev, xdp_prog, index, err);
return err;
}
EXPORT_SYMBOL_GPL(xdp_do_generic_redirect);
^ permalink raw reply related
* [V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage
From: Jesper Dangaard Brouer @ 2017-09-07 12:33 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
Jesper Dangaard Brouer
In-Reply-To: <150478756604.28665.6915020425359475729.stgit@firesoul>
Catch different invalid XDP_REDIRECT and bpf_redirect_map API usage.
It is fairly easy to create a dangling redirect_info->map pointer,
which (until John or Daniel fix this) can crash the kernel.
The intended usage of the BPF helper bpf_redirect_map(), is to return
XDP_REDIRECT action after invoking it, but there is nothing stopping
the bpf_prog to return anything else. When XDP_REDIRECT isn't
returned, then a dangling ->map pointer is left behind, as
xdp_do_redirect() isn't called.
This also happens for drivers not implementing XDP_REDIRECT, as they
are not aware of this new XDP_REDIRECT return code, they leave the map
pointer dangling.
The simply solution to check for a dangling ->map pointer after each
driver napi->poll() invocation, see xdp_do_map_check().
This patch also add a check for a dangling ->map_to_flush pointer.
This should be considered a driver bug, as the driver contract is that
a pair of xdp_do_redirect and xdp_do_flush_map MUST be called in the
same cpu context.
Note, we need to check after each drivers napi->poll call, as:
1. DevA poll call bpf_redirect_map() but not xdp_do_redirect()
2. DevB bpf_prog uses bpf_redirect() and call xdp_do_redirect()
which now use map from DevA
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
include/linux/filter.h | 1 +
net/core/dev.c | 3 +++
net/core/filter.c | 25 +++++++++++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d29e58fde364..0c48941e0022 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -724,6 +724,7 @@ int xdp_do_redirect(struct net_device *dev,
struct xdp_buff *xdp,
struct bpf_prog *prog);
void xdp_do_flush_map(void);
+void xdp_do_map_check(struct napi_struct *napi);
void bpf_warn_invalid_xdp_action(u32 act);
void bpf_warn_invalid_xdp_redirect(u32 ifindex);
diff --git a/net/core/dev.c b/net/core/dev.c
index 6f845e4fec17..7eac642b469f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5320,6 +5320,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock)
*/
rc = napi->poll(napi, BUSY_POLL_BUDGET);
trace_napi_poll(napi, rc, BUSY_POLL_BUDGET);
+ xdp_do_map_check(napi);
netpoll_poll_unlock(have_poll_lock);
if (rc == BUSY_POLL_BUDGET)
__napi_schedule(napi);
@@ -5367,6 +5368,7 @@ void napi_busy_loop(unsigned int napi_id,
}
work = napi_poll(napi, BUSY_POLL_BUDGET);
trace_napi_poll(napi, work, BUSY_POLL_BUDGET);
+ xdp_do_map_check(napi);
count:
if (work > 0)
__NET_ADD_STATS(dev_net(napi->dev),
@@ -5529,6 +5531,7 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll)
if (test_bit(NAPI_STATE_SCHED, &n->state)) {
work = n->poll(n, weight);
trace_napi_poll(n, work, weight);
+ xdp_do_map_check(n);
}
WARN_ON_ONCE(work > weight);
diff --git a/net/core/filter.c b/net/core/filter.c
index 3767470cab6c..f0e1135eeb9d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2500,6 +2500,31 @@ void xdp_do_flush_map(void)
}
EXPORT_SYMBOL_GPL(xdp_do_flush_map);
+void xdp_do_map_check(struct napi_struct *napi)
+{
+ struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+
+ /* XDP drivers (and XDP-generic) must invoke xdp_do_redirect()
+ * when bpf_prog use helper bpf_redirect_map(), else the map
+ * pointer can be left dangling. Catch this invalid API
+ * usage, instead of potentially crashing.
+ */
+ if (ri->map) {
+ ri->map = NULL;
+ net_err_ratelimited("%s: caught invalid XDP bpf_redirect_map\n",
+ napi->dev->name);
+ trace_xdp_exception(napi->dev, NULL, XDP_REDIRECT);
+ }
+ if (ri->map_to_flush) { /* Driver bug */
+ net_err_ratelimited("%s: XDP driver miss xdp_do_flush_map\n",
+ napi->dev->name);
+ trace_xdp_exception(napi->dev, NULL, XDP_REDIRECT);
+ /* Flush map, else pkts can be stuck on XDP TXq */
+ xdp_do_flush_map();
+ }
+}
+EXPORT_SYMBOL_GPL(xdp_do_map_check);
+
static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
^ permalink raw reply related
* Re: [PATCH] iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Kalle Valo @ 2017-09-07 12:39 UTC (permalink / raw)
To: torvalds, Luca Coelho
Cc: linux-wireless, johannes, linux-kernel, akpm, netdev, davem,
emmanuel.grumbach, Luca Coelho
In-Reply-To: <20170907075152.4522-1-luca@coelho.fi>
Luca Coelho <luca@coelho.fi> writes:
> From: Luca Coelho <luciano.coelho@intel.com>
>
> The LEDS_CMD command is only supported in some newer FW versions
> (e.g. iwlwifi-8000C-31.ucode), so we can't send it to older versions
> (such as iwlwifi-8000C-27.ucode).
>
> To fix this, check for a new bit in the FW capabilities TLV that tells
> when the command is supported.
>
> Note that the current version of -31.ucode in linux-firmware.git
> (31.532993.0) does not have this capability bit set, so the LED won't
> work, even though this version should support it. But we will update
> this firmware soon, so it won't be a problem anymore.
>
> Fixes: 7089ae634c50 ("iwlwifi: mvm: use firmware LED command where applicable")
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Linus, do you want to apply this directly or should we take it via the
normal route (wireless-drivers -> net)? If your prefer the latter when
I'm planning to submit this to Dave in a day or two and expecting it to
get to your tree in about a week, depending of course what is Dave's
schedule.
--
Kalle Valo
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-07 12:40 UTC (permalink / raw)
To: Henrik Austad
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano,
jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170907053411.GA6580@sisyphus.home.austad.us>
On Thu, Sep 07, 2017 at 07:34:11AM +0200, Henrik Austad wrote:
> Also, does this mean that when you create the qdisc, you have locked the
> bandwidth for the scheduler? Meaning, if I later want to add another
> stream that requires more bandwidth, I have to close all active streams,
> reconfigure the qdisc and then restart?
No, just allocate enough bandwidth to accomodate all of the expected
streams. The streams can start and stop at will.
> So my understanding of all of this is that you configure the *total*
> bandwith for each class when you load the qdisc and then let userspace
> handle the rest. Is this correct?
Nothing wrong with that.
> In my view, it would be nice if the qdisc had some notion about streams so
> that you could create a stream, feed frames to it and let the driver pace
> them out. (The fewer you queue, the shorter the delay). This will also
> allow you to enforce per-stream bandwidth restrictions. I don't see how you
> can do this here unless you want to do this in userspace.
>
> Do you have any plans for adding support for multiplexing streams? If you
> have multiple streams, how do you enforce that one stream does not eat into
> the bandwidth of another stream? AFAIK, this is something the network must
> enforce, but I see no option of doing som here.
Please, lets keep this simple. Today we have exactly zero user space
applications using this kind of bandwidth reservation. The case of
wanting the kernel to police individual stream usage does not exist,
and probably never will.
For serious TSN use cases, the bandwidth needed by each system and
indeed the entire network will be engineered, and we can reasonably
expect applications to cooperate in this regard.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 0/2] net: Fix crashes due to activity during suspend
From: Florian Fainelli @ 2017-09-07 13:09 UTC (permalink / raw)
To: Geert Uytterhoeven, andrew
Cc: marc_gonzalez, slash.tmp, Geert Uytterhoeven, David S . Miller,
Steve Glendinning, Lukas Wunner, Rafael J . Wysocki,
netdev@vger.kernel.org, Linux PM list, Linux-Renesas,
linux-kernel@vger.kernel.org
In-Reply-To: <5a1b6f23-6fce-1a68-f9c3-ca0d709979e7@gmail.com>
On 08/23/2017 10:13 AM, Florian Fainelli wrote:
> On 08/23/2017 04:45 AM, Geert Uytterhoeven wrote:
>> Hi Florian,
>>
>> On Tue, Aug 22, 2017 at 8:49 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 08/22/2017 11:37 AM, Geert Uytterhoeven wrote:
>>>> If an Ethernet device is used while the device is suspended, the system may
>>>> crash.
>>>>
>>>> E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
>>>> driven by a PM controlled clock. If the Ethernet registers are accessed
>>>> while the clock is not running, the system will crash with an imprecise
>>>> external abort.
>>>>
>>>> This patch series fixes two of such crashes:
>>>> 1. The first patch prevents the PHY polling state machine from accessing
>>>> PHY registers while a device is suspended,
>>>> 2. The second patch prevents the net core from trying to transmit packets
>>>> when an smsc911x device is suspended.
>>>>
>>>> Both crashes can be reproduced on sh73a0/kzm9g and r8a73a4/ape6evm during
>>>> s2ram (rarely), or by using pm_test (more likely to trigger):
>>>>
>>>> # echo 0 > /sys/module/printk/parameters/console_suspend
>>>> # echo platform > /sys/power/pm_test
>>>> # echo mem > /sys/power/state
>>>>
>>>> With this series applied, my test systems survive a loop of 100 test
>>>> suspends.
>>>
>>> It seems to me like part, if not the entire problem is that smsc91xx's
>>> suspend and resume functions are way too simplistic and absolutely do
>>> not manage the PHY during suspend/resume, the PHY state machine is not
>>> even stopped, so of course, this will cause bus errors if you access
>>> those registers.
>>>
>>> You are addressing this as part of patch 2, but this seems to me like
>>> this is still a bit incomplete and you'd need at least phy_stop() and/or
>>> phy_suspend() (does a power down of the PHY) and phy_start() and/or
>>> phy_resume() calls to complete the PHY state machine shutdown during
>>> suspend.
>>>
>>> Have you tried that?
>>
>> Unfortunately that doesn't help.
>> In state PHY_HALTED, the PHY state machine still calls the .adjust_link()
>> callback while the device is suspended.
>
> Humm that is correct yes.
>
>>
>> Do you have a clue? This is too far beyond my phy-foo...
>
> I was initially contemplating a revert of
> 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: Correctly process
> PHY_HALTED in phy_stop_machine()") but this is not the root of the
> problem. The problem really is that phy_stop() does not wait for the PHY
> state machine to be stopped so you cannot rely on that and past the
> function return be offered any guarantees that adjust_link is not called.
>
> We seem to be getting away with that in most drivers because when we see
> phydev->link = 0, we either do nothing or actually turn of the HW block.
>
> How about we export phy_stop_machine() to drivers which would provide a
> synchronization point that would ensure that no HW accesses are done
> past this point?
>
> I am absolutely not clear on the implications of using a freezable
> workqueue with respect to the PHY state machine and how devices are
> going to wind-up being powered down or not...
Geert, as you may have notice a revert of the change was sent so 4.13
should be fine, but ultimately I would like to put the non-reverted code
back in after we add a few safeguards:
- David Daney reported he could crash the kernel by calling just
phy_disconnect() with no prior phy_stop() which is not "legal" but
should not crash either
- and you reported the bus errors on smsc911x when we call adjust_link
during suspend, and due to a lack of hard synchronization so phy_stop()
here does not give you enough guarantees to let you turn off power to
the smsc911x block
If that seems accurate then we can work on something that should be
working again (famous last words).
Thanks
--
Florian
^ permalink raw reply
* Re:
From: Quick Loan @ 2017-09-07 13:34 UTC (permalink / raw)
Hello dear I am an International loan lender, I give out loans at 1% interest
rate, email me at:(rich_ken2016@usa.com)
^ permalink raw reply
* Re: nfp bpf offload add/replace
From: Jakub Kicinski @ 2017-09-07 13:44 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, mlxsw, Daniel Borkmann, Simon Horman
In-Reply-To: <20170907091033.GD1967@nanopsycho>
On Thu, 7 Sep 2017 11:10:33 +0200, Jiri Pirko wrote:
> Hi Kuba.
>
> I'm looking into cls_bpf code and nfp_net_bpf_offload function in your
> driver. Why do you need TC_CLSBPF_ADD? Seems like TC_CLSBPF_REPLACE
> should be enough. It would make the cls_bpf code easier.
>
> Note that other cls just have replace/destroy (u32 too, as drivers
> handle NEW/REPLACE in one switch-case - will patch this).
Could we clarify what the REPLACE is actually supposed to do? :)
In the flower code and the REPLACE looks a lot like ADD on the
surface... If change is called it will invoke REPLACE with the new
filter and then if there was an old filter, it will do DELETE. Is my
understanding correct?
If so I found this model of operation somehow confusing. Plus the
management of flows may get slightly tricky if there is a possibility of
"replacing" a flow with an identical one. Flower may make calls like
these:
add flower vlan_id 100 action ...
# REPLACE vid 100 ...
change ... flower vlan_id 100 action ...
# REPLACE vid 100 ...
# DELETE vid 100 ...
Doesn't this force driver/HW to implement refcounting on the rules?
On why I need the replace - BPF unlike other classifiers usually
installs a single program, I think offloading multiple TC filters is
questionable (people will use tailcalls instead most likely). I want to
be able to implement atomic replace of that single program (i.e. not ADD
followed by DELETE) because that simplifies the driver quite a bit.
^ permalink raw reply
* Re: [PATCH net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Andrew Lunn @ 2017-09-07 13:56 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev, steve.glendinning
In-Reply-To: <CE371C1263339941885964188A0225FA3339DB@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 06:51:37AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Configure pause time to 0xffff when tx flow control enabled
Hi Nisar
You should explain the 'Why' in the commit message. Why do we want a
pause time of 0xffff?
Andrew
^ permalink raw reply
* Re: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Andrew Lunn @ 2017-09-07 13:59 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A58@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> This series of patches are for lan78xx driver.
>
> These patches fixes potential issues associated with lan78xx driver
Hi Nisar
So this is version 2? Please include v2 in the subject line.
Also, briefly list what is different from the previous version.
Andrew
^ permalink raw reply
* RE: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Woojung.Huh @ 2017-09-07 14:03 UTC (permalink / raw)
To: andrew, Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <20170907135909.GH11248@lunn.ch>
> On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com
> wrote:
> > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> >
> > This series of patches are for lan78xx driver.
> >
> > These patches fixes potential issues associated with lan78xx driver
>
> Hi Nisar
>
> So this is version 2? Please include v2 in the subject line.
>
> Also, briefly list what is different from the previous version.
Hi Andrew,
Because Nisar dropped of non-mdio patch in the series,
I suggested to treat as new patch than version 2.
In this case, it is still considered as version 2 than new series?
- Woojung
^ permalink raw reply
* Re: [PATCH net 1/3] lan78xx: Fix for eeprom read/write when device autosuspend
From: Andrew Lunn @ 2017-09-07 14:04 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A6B@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:06AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Fix for eeprom read/write when device autosuspend
So this is a fix? And you said [PATCH net 1/3], meaning it is supposed
to go into stable? You should try to add a fixes tag. Something like:
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Andrew
^ permalink raw reply
* Re: nfp bpf offload add/replace
From: Jiri Pirko @ 2017-09-07 14:05 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, mlxsw, Daniel Borkmann, Simon Horman
In-Reply-To: <20170907144412.7a4a7cf7@cakuba.netronome.com>
Thu, Sep 07, 2017 at 03:44:12PM CEST, kubakici@wp.pl wrote:
>On Thu, 7 Sep 2017 11:10:33 +0200, Jiri Pirko wrote:
>> Hi Kuba.
>>
>> I'm looking into cls_bpf code and nfp_net_bpf_offload function in your
>> driver. Why do you need TC_CLSBPF_ADD? Seems like TC_CLSBPF_REPLACE
>> should be enough. It would make the cls_bpf code easier.
>>
>> Note that other cls just have replace/destroy (u32 too, as drivers
>> handle NEW/REPLACE in one switch-case - will patch this).
>
>Could we clarify what the REPLACE is actually supposed to do? :)
>
>In the flower code and the REPLACE looks a lot like ADD on the
>surface... If change is called it will invoke REPLACE with the new
>filter and then if there was an old filter, it will do DELETE. Is my
>understanding correct?
Yes, correct.
>
>If so I found this model of operation somehow confusing. Plus the
>management of flows may get slightly tricky if there is a possibility of
>"replacing" a flow with an identical one. Flower may make calls like
>these:
>
>add flower vlan_id 100 action ...
># REPLACE vid 100 ...
>change ... flower vlan_id 100 action ...
># REPLACE vid 100 ...
># DELETE vid 100 ...
Yes, that is the flow.
>
>Doesn't this force driver/HW to implement refcounting on the rules?
Why do you think so? There is a cookie that is passed from flower down
and driver uses it to remove the entry.
>
>On why I need the replace - BPF unlike other classifiers usually
>installs a single program, I think offloading multiple TC filters is
>questionable (people will use tailcalls instead most likely). I want to
>be able to implement atomic replace of that single program (i.e. not ADD
>followed by DELETE) because that simplifies the driver quite a bit.
Understood. So, looks like the REPLACE/DESTROY would be sufficient for
bpf. ADD is not needed as it can be done by REPLACE-NULL, right?
On the other hand, the rest of the cls, namely flower, u32 and matchall
need ADD/DESTROY as they don't really do no replacing.
Makes sense?
^ permalink raw reply
* Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP
From: Daniel Borkmann @ 2017-09-07 14:09 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
alexei.starovoitov
In-Reply-To: <150478759310.28665.17184783248584070473.stgit@firesoul>
On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
> Using bpf_redirect_map is allowed for generic XDP programs, but the
> appropriate map lookup was never performed in xdp_do_generic_redirect().
>
> Instead the map-index is directly used as the ifindex. For the
> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
> sending on ifindex 0 which isn't valid, resulting in getting SKB
> packets dropped. Thus, the reported performance numbers are wrong in
> commit 24251c264798 ("samples/bpf: add option for native and skb mode
> for redirect apps") for the 'xdp_redirect_map -S' case.
>
> It might seem innocent this was lacking, but it can actually crash the
> kernel. The potential crash is caused by not consuming redirect_info->map.
> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map
> pointer, which will survive even after unloading the xdp bpf_prog and
> deallocating the devmap data-structure. This leaves a dead map
> pointer around. The kernel will crash when loading the xdp_redirect
> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
> and returns XDP_REDIRECT, which will cause it to dereference the map
> pointer.
>
> Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect apps")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/trace/events/xdp.h | 4 ++--
> net/core/filter.c | 14 +++++++++++---
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> index 862575ac8da9..4e16c43fba10 100644
> --- a/include/trace/events/xdp.h
> +++ b/include/trace/events/xdp.h
> @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,
>
> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
> trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0, \
> - 0, map, idx);
> + 0, map, idx)
>
> #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \
> trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0, \
> - err, map, idx);
> + err, map, idx)
>
> #endif /* _TRACE_XDP_H */
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5912c738a7b2..3767470cab6c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
> struct bpf_prog *xdp_prog)
> {
> struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> + struct bpf_map *map = ri->map;
> u32 index = ri->ifindex;
> struct net_device *fwd;
> unsigned int len;
> int err = 0;
>
> - fwd = dev_get_by_index_rcu(dev_net(dev), index);
> ri->ifindex = 0;
> + ri->map = NULL;
> +
> + if (map)
> + fwd = __dev_map_lookup_elem(map, index);
> + else
> + fwd = dev_get_by_index_rcu(dev_net(dev), index);
> if (unlikely(!fwd)) {
> err = -EINVAL;
> goto err;
> @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
> }
>
> skb->dev = fwd;
Looks much better above, thanks!
> - _trace_xdp_redirect(dev, xdp_prog, index);
> + map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
> + : _trace_xdp_redirect(dev, xdp_prog, index);
Could we rather make this in a way such that when the two
tracepoints are disabled and thus patched out, that we can
also omit the extra conditional which has no purpose then?
Perhaps just a consolidated _trace_xdp_generic_redirect_map()
would be better to avoid this altogether given we have twice
the same anyway, here and in err path.
Thanks,
Daniel
> return 0;
> err:
> - _trace_xdp_redirect_err(dev, xdp_prog, index, err);
> + map ? _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err)
> + : _trace_xdp_redirect_err(dev, xdp_prog, index, err);
> return err;
> }
> EXPORT_SYMBOL_GPL(xdp_do_generic_redirect);
>
^ permalink raw reply
* Re: [PATCH net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Andrew Lunn @ 2017-09-07 14:10 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A7C@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:26AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Allow EEPROM write for less than MAX_EEPROM_SIZE
>
> Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index baf91c7..94ef943 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1299,6 +1299,10 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == EEPROM_INDICATOR))
return lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
> (ee->len == 512) &&
> (data[0] == OTP_INDICATOR_1))
> ret = lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
> + else if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
> + (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
> + (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE))
> + ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
>
> usb_autopm_put_interface(dev->intf);
Hi Nisar
You should explain why this change cannot be folded into the first if
statement.
Andrew
^ permalink raw reply
* Re: [PATCH 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting
From: Andrew Lunn @ 2017-09-07 14:13 UTC (permalink / raw)
To: Nisar.Sayed; +Cc: davem, UNGLinuxDriver, netdev
In-Reply-To: <CE371C1263339941885964188A0225FA333A8D@CHN-SV-EXMX03.mchp-main.com>
On Thu, Sep 07, 2017 at 07:11:50AM +0000, Nisar.Sayed@microchip.com wrote:
> From: Nisar Sayed <Nisar.Sayed@microchip.com>
>
> Use default value loaded from EEPROM/OTP when resetting
Hi Nisar
Subject: [PATCH 3/3]
Is this a fix for net, or further development for net-next?
Why do we want the default values?
Andrew
^ permalink raw reply
* Re: [V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage
From: Daniel Borkmann @ 2017-09-07 14:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller
Cc: Daniel Borkmann, John Fastabend, Andy Gospodarek,
alexei.starovoitov
In-Reply-To: <150478759820.28665.14031878598812204399.stgit@firesoul>
Hey Jesper,
On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
> Catch different invalid XDP_REDIRECT and bpf_redirect_map API usage.
>
> It is fairly easy to create a dangling redirect_info->map pointer,
> which (until John or Daniel fix this) can crash the kernel.
[...]
Here's what I wrote up yesterday, test looked good, feel
free to give it a spin as well if you want. Was planning
to submit it later today as official patch. Definitely less
intrusive than adding something to napi hot path impacting
everyone.
Cheers,
Daniel
From 56ad381c87dcc2b32156c5970c75bf98818ea7f2 Mon Sep 17 00:00:00 2001
Message-Id: <56ad381c87dcc2b32156c5970c75bf98818ea7f2.1504790124.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 6 Sep 2017 21:21:27 +0200
Subject: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
We can potentially run into a couple of issues with the XDP
bpf_redirect_map() helper. The ri->map in the per CPU storage
can become stale in several ways, mostly due to misuse, where
we can then trigger a use after free on the map:
i) prog A is calling bpf_redirect_map(), returning XDP_REDIRECT
and running on a driver not supporting XDP_REDIRECT yet. The
ri->map on that CPU becomes stale when the XDP program is unloaded
on the driver, and a prog B loaded on a different driver which
supports XDP_REDIRECT return code. prog B would have to omit
calling to bpf_redirect_map() and just return XDP_REDIRECT, which
would then access the freed map in xdp_do_redirect() since not
cleared for that CPU.
ii) prog A is calling bpf_redirect_map(), returning a code other
than XDP_REDIRECT. prog A is then detached, which triggers release
of the map. prog B is attached which, similarly as in i), would
just return XDP_REDIRECT without having called bpf_redirect_map()
and thus be accessing the freed map in xdp_do_redirect() since
not cleared for that CPU.
iii) prog A is attached to generic XDP, calling the bpf_redirect_map()
helper and returning XDP_REDIRECT. xdp_do_generic_redirect() is
currently not handling ri->map (will be fixed by Jesper), so it's
not being reset. Later loading a e.g. native prog B which would,
say, call bpf_xdp_redirect() and then returns XDP_REDIRECT would
find in xdp_do_redirect() that a map was set and uses that causing
use after free on map access.
Fix thus needs to avoid accessing stale ri->map pointers, naive
way would be to call a BPF function from drivers that just resets
it to NULL for all XDP return codes but XDP_REDIRECT and including
XDP_REDIRECT for drivers not supporting it yet (and let ri->map
being handled in xdp_do_generic_redirect()). There is a less
intrusive way w/o letting drivers call a reset for each BPF run.
The verifier knows we're calling into bpf_xdp_redirect_map()
helper, so it can do a small insn rewrite transparent to the prog
itself in the sense that it fills R4 with a pointer to the own
bpf_prog. We have that pointer at verification time anyway and
R4 is allowed to be used as per calling convention we scratch
R0 to R5 anyway, so they become inaccessible and program cannot
read them prior to a write. Then, the helper would store the prog
pointer in the current CPUs struct redirect_info. Later in
xdp_do_*_redirect() we check whether the redirect_info's prog
pointer is the same as passed xdp_prog pointer, and if that's
the case then all good, since the prog holds a ref on the map
anyway, so it is always valid at that point in time and must
have a reference count of at least 1. If in the unlikely case
they are not equal, it means we got a stale pointer, so we clear
and bail out right there. Also do reset map and the owning prog
in bpf_xdp_redirect(), so that bpf_xdp_redirect_map() and
bpf_xdp_redirect() won't get mixed up, only the last call should
take precedence. A tc bpf_redirect() doesn't use map anywhere
yet, so no need to clear it there since never accessed in that
layer.
Note that in case the prog is released, and thus the map as
well we're still under RCU read critical section at that time
and have preemption disabled as well. Once we commit with the
__dev_map_insert_ctx() from xdp_do_redirect_map() and set the
map to ri->map_to_flush, we still wait for a xdp_do_flush_map()
to finish in devmap dismantle time once flush_needed bit is set,
so that is fine.
Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/verifier.c | 16 ++++++++++++++++
net/core/filter.c | 21 +++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..af13987 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
continue;
}
+ if (insn->imm == BPF_FUNC_redirect_map) {
+ uint64_t addr = (unsigned long)prog;
+ struct bpf_insn r4_ld[] = {
+ BPF_LD_IMM64(BPF_REG_4, addr),
+ *insn,
+ };
+ cnt = ARRAY_SIZE(r4_ld);
+
+ new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ }
patch_call_imm:
fn = prog->aux->ops->get_func_proto(insn->imm);
/* all functions that have prototype and verifier allowed
diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c73..0848df2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1794,6 +1794,7 @@ struct redirect_info {
u32 flags;
struct bpf_map *map;
struct bpf_map *map_to_flush;
+ const struct bpf_prog *map_owner;
};
static DEFINE_PER_CPU(struct redirect_info, redirect_info);
@@ -1807,7 +1808,6 @@ struct redirect_info {
ri->ifindex = ifindex;
ri->flags = flags;
- ri->map = NULL;
return TC_ACT_REDIRECT;
}
@@ -2504,6 +2504,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ const struct bpf_prog *map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
@@ -2511,6 +2512,15 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
ri->ifindex = 0;
ri->map = NULL;
+ ri->map_owner = NULL;
+
+ /* This is really only caused by a deliberately crappy
+ * BPF program, normally we would never hit that case,
+ * so no need to inform someone via tracepoints either,
+ * just bail out.
+ */
+ if (unlikely(map_owner != xdp_prog))
+ return -EINVAL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd) {
@@ -2607,6 +2617,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
+ ri->map = NULL;
+ ri->map_owner = NULL;
return XDP_REDIRECT;
}
@@ -2619,7 +2631,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
.arg2_type = ARG_ANYTHING,
};
-BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags)
+BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
+ const struct bpf_prog *, map_owner)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
@@ -2629,10 +2642,14 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
ri->map = map;
+ ri->map_owner = map_owner;
return XDP_REDIRECT;
}
+/* Note, arg4 is hidden from users and populated by the verifier
+ * with the right pointer.
+ */
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
.func = bpf_xdp_redirect_map,
.gpl_only = false,
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] netfilter: xt_hashlimit: avoid 64-bit division
From: Geert Uytterhoeven @ 2017-09-07 14:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Vishwanath Pai, Josh Hunt, netfilter-devel,
coreteam, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170906195825.3715290-1-arnd@arndb.de>
Hi Arnd,
On Wed, Sep 6, 2017 at 9:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 64-bit division is expensive on 32-bit architectures, and
> requires a special function call to avoid a link error like:
>
> net/netfilter/xt_hashlimit.o: In function `hashlimit_mt_common':
> xt_hashlimit.c:(.text+0x1328): undefined reference to `__aeabi_uldivmod'
>
> In the case of hashlimit_mt_common, we don't actually need a
> 64-bit operation, we can simply rewrite the function slightly
> to make that clear to the compiler.
>
> Fixes: bea74641e378 ("netfilter: xt_hashlimit: add rate match mode")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks, this fixes a similar issue (__udivdi3 undefined) on m68k.
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH net 0/3] lan78xx: Fixes to lan78xx driver
From: Andrew Lunn @ 2017-09-07 14:19 UTC (permalink / raw)
To: Woojung.Huh; +Cc: Nisar.Sayed, davem, UNGLinuxDriver, netdev
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B1ADB7@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 02:03:38PM +0000, Woojung.Huh@microchip.com wrote:
> > On Thu, Sep 07, 2017 at 07:10:51AM +0000, Nisar.Sayed@microchip.com
> > wrote:
> > > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> > >
> > > This series of patches are for lan78xx driver.
> > >
> > > These patches fixes potential issues associated with lan78xx driver
> >
> > Hi Nisar
> >
> > So this is version 2? Please include v2 in the subject line.
> >
> > Also, briefly list what is different from the previous version.
> Hi Andrew,
>
> Because Nisar dropped of non-mdio patch in the series,
> I suggested to treat as new patch than version 2.
> In this case, it is still considered as version 2 than new series?
Hi Woojung
So it is not a clear hard rule here. But we have seen these patches
before, and they have been modified based on my comments. So i would
say these are version 2. But a new series would also be O.K. What
really matters is that the cover note explains what is going on. That
some of the patches in the previous version have been dropped and will
be posted later, and what changes have been made to the remaining
patches.
Andrew
^ permalink raw reply
* [PATCH iproute2 4/4] devlink: Add support for protocol IPv4/IPv6/Ethernet special formats
From: Arkadi Sharshevsky @ 2017-09-07 14:26 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, jiri, mlxsw, andrew, Arkadi Sharshevsky,
Jiri Pirko
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
Add support for protocol IPv4/IPv6/Ethernet special formats.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index b87de38..39cda06 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -20,6 +20,7 @@
#include <linux/genetlink.h>
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
+#include <netinet/ether.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -3401,7 +3402,79 @@ struct dpipe_header_printer {
unsigned int header_id;
};
-static struct dpipe_header_printer *dpipe_header_printers[] = {};
+static void dpipe_field_printer_ipv4_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ struct in_addr ip_addr;
+
+ ip_addr.s_addr = htonl(*(uint32_t *)value);
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type), inet_ntoa(ip_addr));
+}
+
+static void
+dpipe_field_printer_ethernet_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type),
+ ether_ntoa((struct ether_addr *)value));
+}
+
+static void dpipe_field_printer_ipv6_addr(struct dpipe_ctx *ctx,
+ enum dpipe_value_type type,
+ void *value)
+{
+ char str[INET6_ADDRSTRLEN];
+
+ inet_ntop(AF_INET6, value, str, INET6_ADDRSTRLEN);
+ pr_out_str(ctx->dl, dpipe_value_type_e2s(type), str);
+}
+
+static struct dpipe_field_printer dpipe_field_printers_ipv4[] = {
+ {
+ .printer = dpipe_field_printer_ipv4_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
+ }
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ipv4 = {
+ .printers = dpipe_field_printers_ipv4,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ipv4),
+ .header_id = DEVLINK_DPIPE_HEADER_IPV4,
+};
+
+static struct dpipe_field_printer dpipe_field_printers_ethernet[] = {
+ {
+ .printer = dpipe_field_printer_ethernet_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_ETHERNET_DST_MAC,
+ },
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ethernet = {
+ .printers = dpipe_field_printers_ethernet,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ethernet),
+ .header_id = DEVLINK_DPIPE_HEADER_ETHERNET,
+};
+
+static struct dpipe_field_printer dpipe_field_printers_ipv6[] = {
+ {
+ .printer = dpipe_field_printer_ipv6_addr,
+ .field_id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+ }
+};
+
+static struct dpipe_header_printer dpipe_header_printer_ipv6 = {
+ .printers = dpipe_field_printers_ipv6,
+ .printers_count = ARRAY_SIZE(dpipe_field_printers_ipv6),
+ .header_id = DEVLINK_DPIPE_HEADER_IPV6,
+};
+
+static struct dpipe_header_printer *dpipe_header_printers[] = {
+ &dpipe_header_printer_ipv4,
+ &dpipe_header_printer_ethernet,
+ &dpipe_header_printer_ipv6,
+};
static int dpipe_print_prot_header(struct dpipe_ctx *ctx,
struct dpipe_op_info *info,
--
2.4.11
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox