* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Y Song @ 2019-07-01 18:39 UTC (permalink / raw)
To: Song Liu
Cc: Jiri Benc, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
In-Reply-To: <CAPhsuW4Ric_nMGxpKf3mEJw3JDBZYpbeAQwTW_Nrsz79T2zisw@mail.gmail.com>
On Sat, Jun 29, 2019 at 11:05 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> > >
> > > Selftests are reporting this failure in test_lwt_seg6local.sh:
> > >
> > > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > > Error fetching program/map!
> > > Failed to parse eBPF program: Operation not permitted
> > >
> > > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > > clang from inserting those functions in .text. In that case, .text is not
> > > marked as relocateable.
> > >
> > > See the output of objdump -h test_lwt_seg6local.o:
> > >
> > > Idx Name Size VMA LMA File off Algn
> > > 0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
> > > CONTENTS, ALLOC, LOAD, READONLY, CODE
> > >
> > > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > > relocateable .text section in the file.
> > >
> > > Add 'static inline' to fix this.
> > >
> > > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > > Signed-off-by: Jiri Benc <jbenc@redhat.com>
> >
> > Maybe use "__always_inline" as most other tests do?
>
> I meant "static __always_inline".
By default, we have
# define __always_inline inline __attribute__((always_inline))
So just use __always_inline should be less verbose in your patch.
BTW, what compiler did you use have this behavior?
Did you have issues with `static __attribute__((always_inline))`?
>
> Song
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Stanislav Fomichev @ 2019-07-01 18:38 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, Stanislav Fomichev, netdev@vger.kernel.org,
bpf@vger.kernel.org, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net, Andrii Nakryiko, kernel test robot
In-Reply-To: <a66c937f-94c0-eaf8-5b37-8587d66c0c62@fb.com>
On 07/01, Yonghong Song wrote:
>
>
> On 7/1/19 9:04 AM, Stanislav Fomichev wrote:
> > On 07/01, Andrii Nakryiko wrote:
> >> On Sat, Jun 29, 2019 at 10:53 PM Yonghong Song <yhs@fb.com> wrote:
> >>>
> >>>
> >>>
> >>> On 6/28/19 4:10 PM, Stanislav Fomichev wrote:
> >>>> Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> >>>> that it can do a single u64 store into user_ip6[2] instead of two
> >>>> separate u32 ones:
> >>>>
> >>>> # 17: (18) r2 = 0x100000000000000
> >>>> # ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> >>>> # 19: (7b) *(u64 *)(r1 +16) = r2
> >>>> # invalid bpf_context access off=16 size=8
> >>>>
> >>>> From the compiler point of view it does look like a correct thing
> >>>> to do, so let's support it on the kernel side.
> >>>>
> >>>> Credit to Andrii Nakryiko for a proper implementation of
> >>>> bpf_ctx_wide_store_ok.
> >>>>
> >>>> Cc: Andrii Nakryiko <andriin@fb.com>
> >>>> Cc: Yonghong Song <yhs@fb.com>
> >>>> Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> >>>> Reported-by: kernel test robot <rong.a.chen@intel.com>
> >>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >>>
> >>> The change looks good to me with the following nits:
> >>> 1. could you add a cover letter for the patch set?
> >>> typically if the number of patches is more than one,
> >>> it would be a good practice with a cover letter.
> >>> See bpf_devel_QA.rst .
> >>> 2. with this change, the comments in uapi bpf.h
> >>> are not accurate any more.
> >>> __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> >>> * Stored in network byte order.
> >>>
> >>> */
> >>> __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> >>> * Stored in network byte order.
> >>> */
> >>> now for stores, aligned 8-byte write is permitted.
> >>> could you update this as well?
> >>>
> >>> From the typical usage pattern, I did not see a need
> >>> for 8-tye read of user_ip6 and msg_src_ip6 yet. So let
> >>> us just deal with write for now.
> >>
> >> But I guess it's still possible for clang to optimize two consecutive
> >> 4-byte reads into single 8-byte read in some circumstances? If that's
> >> the case, maybe it's a good idea to have corresponding read checks as
> >> well?
> > I guess clang can do those kinds of optimizations. I can put it on my
> > todo and address later (or when we actually see it out in the wild).
>
> Okay, I find a Facebook internal app. does trying to read the 4 bytes
> and compare to a predefined loopback address. We may need to handle
> read cases as well. But this can be a followup after actual tryout.
Sounds good, will follow up on that.
> >
> >> But overall this looks good to me:
> >>
> >> Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Thanks for a review!
> >
> >>>
> >>> With the above two nits,
> >>> Acked-by: Yonghong Song <yhs@fb.com>
> >>>
> >>>> ---
> >>>> include/linux/filter.h | 6 ++++++
> >>>> net/core/filter.c | 22 ++++++++++++++--------
> >>>> 2 files changed, 20 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/filter.h b/include/linux/filter.h
> >>>> index 340f7d648974..3901007e36f1 100644
> >>>> --- a/include/linux/filter.h
> >>>> +++ b/include/linux/filter.h
> >>>> @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> >>>> return size <= size_default && (size & (size - 1)) == 0;
> >>>> }
> >>>>
> >>>> +#define bpf_ctx_wide_store_ok(off, size, type, field) \
> >>>> + (size == sizeof(__u64) && \
> >>>> + off >= offsetof(type, field) && \
> >>>> + off + sizeof(__u64) <= offsetofend(type, field) && \
> >>>> + off % sizeof(__u64) == 0)
> >>>> +
> >>>> #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
> >>>>
> >>>> static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> >>>> diff --git a/net/core/filter.c b/net/core/filter.c
> >>>> index dc8534be12fc..5d33f2146dab 100644
> >>>> --- a/net/core/filter.c
> >>>> +++ b/net/core/filter.c
> >>>> @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> >>>> if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> >>>> return false;
> >>>> } else {
> >>>> + if (bpf_ctx_wide_store_ok(off, size,
> >>>> + struct bpf_sock_addr,
> >>>> + user_ip6))
> >>>> + return true;
> >>>> +
> >>>> + if (bpf_ctx_wide_store_ok(off, size,
> >>>> + struct bpf_sock_addr,
> >>>> + msg_src_ip6))
> >>>> + return true;
> >>>> +
> >>>> if (size != size_default)
> >>>> return false;
> >>>> }
> >>>> @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>> /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> >>>> * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> >>>> *
> >>>> - * It doesn't support SIZE argument though since narrow stores are not
> >>>> - * supported for now.
> >>>> - *
> >>>> * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> >>>> * "register" since two registers available in convert_ctx_access are not
> >>>> * enough: we can't override neither SRC, since it contains value to store, nor
> >>>> @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>> * instructions. But we need a temporary place to save pointer to nested
> >>>> * structure whose field we want to store to.
> >>>> */
> >>>> -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
> >>>> +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
> >>>> do { \
> >>>> int tmp_reg = BPF_REG_9; \
> >>>> if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
> >>>> @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>> offsetof(S, TF)); \
> >>>> *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
> >>>> si->dst_reg, offsetof(S, F)); \
> >>>> - *insn++ = BPF_STX_MEM( \
> >>>> - BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
> >>>> + *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
> >>>> bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
> >>>> target_size) \
> >>>> + OFF); \
> >>>> @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>> TF) \
> >>>> do { \
> >>>> if (type == BPF_WRITE) { \
> >>>> - SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
> >>>> - TF); \
> >>>> + SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
> >>>> + OFF, TF); \
> >>>> } else { \
> >>>> SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
> >>>> S, NS, F, NF, SIZE, OFF); \
> >>>>
^ permalink raw reply
* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Miller @ 2019-07-01 18:32 UTC (permalink / raw)
To: mcroce; +Cc: netdev, linux-kernel, kuznet, yoshfuji
In-Reply-To: <20190701170155.1967-1-mcroce@redhat.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Mon, 1 Jul 2019 19:01:55 +0200
> Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
>
> # ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global noprefixroute dummy0
> valid_lft forever preferred_lft forever
>
> Or worse, by sending a malicious netlink command:
>
> # ip -4 addr show dev dummy0
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
> valid_lft forever preferred_lft forever
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Applied and queued up for -stable, thanks Matteo.
^ permalink raw reply
* [RESEND PATCH iproute2 net-next] devlink: Introduce PCI PF and VF port flavour and attribute
From: Parav Pandit @ 2019-07-01 18:30 UTC (permalink / raw)
To: netdev; +Cc: saeedm, jakub.kicinski, jiri, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>
Introduce PCI PF and VF port flavour and port attributes such as PF
number and VF number.
$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
devlink/devlink.c | 23 +++++++++++++++++++++++
include/uapi/linux/devlink.h | 11 +++++++++++
2 files changed, 34 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 559f624e..15493426 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2771,6 +2771,10 @@ static const char *port_flavour_name(uint16_t flavour)
return "cpu";
case DEVLINK_PORT_FLAVOUR_DSA:
return "dsa";
+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
+ return "pcipf";
+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
+ return "pcivf";
default:
return "<unknown flavour>";
}
@@ -2803,8 +2807,27 @@ static void pr_out_port(struct dl *dl, struct nlattr **tb)
if (tb[DEVLINK_ATTR_PORT_FLAVOUR]) {
uint16_t port_flavour =
mnl_attr_get_u16(tb[DEVLINK_ATTR_PORT_FLAVOUR]);
+ uint16_t pf_vf;
pr_out_str(dl, "flavour", port_flavour_name(port_flavour));
+ if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+ if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+ pr_out_uint(dl, "pfnum", pf_vf);
+ }
+ } else if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+ if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+ pr_out_uint(dl, "pfnum", pf_vf);
+ }
+ if (tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]);
+ pr_out_uint(dl, "vfnum", pf_vf);
+ }
+ }
}
if (tb[DEVLINK_ATTR_PORT_SPLIT_GROUP])
pr_out_uint(dl, "split_group",
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6544824a..fc195cbd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,14 @@ enum devlink_port_flavour {
DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
* interconnect port.
*/
+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+ * the PCI PF. It is an internal
+ * port that faces the PCI PF.
+ */
+ DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+ * for the PCI VF. It is an internal
+ * port that faces the PCI VF.
+ */
};
enum devlink_param_cmode {
@@ -337,6 +345,9 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
+ DEVLINK_ATTR_PORT_PCI_VF_NUMBER, /* u16 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net-next 1/7] net/rds: Give fr_state a chance to transition to FRMR_IS_FREE
From: David Miller @ 2019-07-01 18:27 UTC (permalink / raw)
To: gerd.rausch; +Cc: santosh.shilimkar, netdev
In-Reply-To: <44f1794c-7c9c-35bc-dc64-a2a993d06a6e@oracle.com>
From: Gerd Rausch <gerd.rausch@oracle.com>
Date: Mon, 1 Jul 2019 09:39:44 -0700
> + /* Memory regions make it onto the "clean_list" via
> + * "rds_ib_flush_mr_pool", after the memory region has
> + * been posted for invalidation via "rds_ib_post_inv".
> + *
> + * At that point in time, "fr_state" may still be
> + * in state "FRMR_IS_INUSE", since the only place where
> + * "fr_state" transitions to "FRMR_IS_FREE" is in
> + * is in "rds_ib_mr_cqe_handler", which is
> + * triggered by a tasklet.
> + *
> + * So in case we notice that
> + * "fr_state != FRMR_IS_FREE" (see below), * we wait for
> + * "fr_inv_done" to trigger with a maximum of 10msec.
> + * Then we check again, and only put the memory region
> + * onto the drop_list (via "rds_ib_free_frmr")
> + * in case the situation remains unchanged.
> + *
> + * This avoids the problem of memory-regions bouncing
> + * between "clean_list" and "drop_list" before they
> + * even have a chance to be properly invalidated.
> + */
> + frmr = &ibmr->u.frmr;
> + wait_event_timeout(frmr->fr_inv_done,
> + frmr->fr_state == FRMR_IS_FREE,
> + msecs_to_jiffies(10));
> + if (frmr->fr_state == FRMR_IS_FREE)
> + break;
If we see FRMR_IS_FREE after the timeout, what cleans this up?
Also, why 10msec? Why that specific value and not some other value? Why
not wait for however long it takes for the tasklet to run and clean it up?
^ permalink raw reply
* [PATCH] devlink: Introduce PCI PF and VF port flavour and attribute
From: Parav Pandit @ 2019-07-01 18:27 UTC (permalink / raw)
To: netdev; +Cc: saeedm, jakub.kicinski, jiri, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>
Introduce PCI PF and VF port flavour and port attributes such as PF
number and VF number.
$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
devlink/devlink.c | 23 +++++++++++++++++++++++
include/uapi/linux/devlink.h | 11 +++++++++++
2 files changed, 34 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 559f624e..15493426 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2771,6 +2771,10 @@ static const char *port_flavour_name(uint16_t flavour)
return "cpu";
case DEVLINK_PORT_FLAVOUR_DSA:
return "dsa";
+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
+ return "pcipf";
+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
+ return "pcivf";
default:
return "<unknown flavour>";
}
@@ -2803,8 +2807,27 @@ static void pr_out_port(struct dl *dl, struct nlattr **tb)
if (tb[DEVLINK_ATTR_PORT_FLAVOUR]) {
uint16_t port_flavour =
mnl_attr_get_u16(tb[DEVLINK_ATTR_PORT_FLAVOUR]);
+ uint16_t pf_vf;
pr_out_str(dl, "flavour", port_flavour_name(port_flavour));
+ if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+ if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+ pr_out_uint(dl, "pfnum", pf_vf);
+ }
+ } else if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+ if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+ pr_out_uint(dl, "pfnum", pf_vf);
+ }
+ if (tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]) {
+ pf_vf = mnl_attr_get_u16(
+ tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]);
+ pr_out_uint(dl, "vfnum", pf_vf);
+ }
+ }
}
if (tb[DEVLINK_ATTR_PORT_SPLIT_GROUP])
pr_out_uint(dl, "split_group",
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6544824a..fc195cbd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,14 @@ enum devlink_port_flavour {
DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
* interconnect port.
*/
+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+ * the PCI PF. It is an internal
+ * port that faces the PCI PF.
+ */
+ DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+ * for the PCI VF. It is an internal
+ * port that faces the PCI VF.
+ */
};
enum devlink_param_cmode {
@@ -337,6 +345,9 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
+ DEVLINK_ATTR_PORT_PCI_VF_NUMBER, /* u16 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Miller @ 2019-07-01 18:22 UTC (permalink / raw)
To: mcroce; +Cc: joe, netdev, linux-kernel, kuznet, yoshfuji
In-Reply-To: <CAGnkfhx9F1G8K6PjBdUnkCO07GR=ktWAnqOLTcOvg7VGwWb69Q@mail.gmail.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Mon, 1 Jul 2019 18:13:32 +0200
> Can this be edidet on patchwork instead of spamming with a v2?
"Spamming"?
It's never spamming, resends make my life SO much easier.
^ permalink raw reply
* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Ahern @ 2019-07-01 18:20 UTC (permalink / raw)
To: Matteo Croce, netdev
Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
In-Reply-To: <c8fac6db-6455-b138-aca9-2f54d782a0b6@gmail.com>
On 7/1/19 11:19 AM, David Ahern wrote:
> I guess at this point we can fail the address add, so this is the best
> option.
bleh, that should be 'can not'
^ permalink raw reply
* Re: [PATCH v2 net-next 14/19] ionic: Add Tx and Rx handling
From: Shannon Nelson @ 2019-07-01 18:17 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629115723.5ae777bc@cakuba.netronome.com>
On 6/29/19 11:57 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:29 -0700, Shannon Nelson wrote:
>> +static int ionic_tx(struct queue *q, struct sk_buff *skb)
>> +{
>> + struct tx_stats *stats = q_to_tx_stats(q);
>> + int err;
>> +
>> + if (skb->ip_summed == CHECKSUM_PARTIAL)
>> + err = ionic_tx_calc_csum(q, skb);
>> + else
>> + err = ionic_tx_calc_no_csum(q, skb);
>> + if (err)
>> + return err;
>> +
>> + err = ionic_tx_skb_frags(q, skb);
>> + if (err)
>> + return err;
>> +
>> + skb_tx_timestamp(skb);
>> + stats->pkts++;
>> + stats->bytes += skb->len;
> Presumably this is 64bit so you should use
> u64_stats_update_begin()
> u64_stats_update_end()
> around it (and all other stats).
Since the device won't work in a 32-bit arch and I have the the Kconfig
set to depend on 64BIT, I wasn't sure I needed to bother with the extra
syntactic sugar.
>> +
>> + ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb);
>> +
>> + return 0;
>> +}
>> +
>> +static int ionic_tx_descs_needed(struct queue *q, struct sk_buff *skb)
>> +{
>> + struct tx_stats *stats = q_to_tx_stats(q);
>> + int err;
>> +
>> + /* If TSO, need roundup(skb->len/mss) descs */
>> + if (skb_is_gso(skb))
>> + return (skb->len / skb_shinfo(skb)->gso_size) + 1;
>> +
>> + /* If non-TSO, just need 1 desc and nr_frags sg elems */
>> + if (skb_shinfo(skb)->nr_frags <= IONIC_TX_MAX_SG_ELEMS)
>> + return 1;
>> +
>> + /* Too many frags, so linearize */
>> + err = skb_linearize(skb);
>> + if (err)
>> + return err;
>> +
>> + stats->linearize++;
>> +
>> + /* Need 1 desc and zero sg elems */
>> + return 1;
>> +}
>> +
>> +netdev_tx_t ionic_start_xmit(struct sk_buff *skb, struct net_device *netdev)
>> +{
>> + u16 queue_index = skb_get_queue_mapping(skb);
>> + struct lif *lif = netdev_priv(netdev);
>> + struct queue *q;
>> + int ndescs;
>> + int err;
>> +
>> + if (unlikely(!test_bit(LIF_UP, lif->state))) {
>> + dev_kfree_skb(skb);
>> + return NETDEV_TX_OK;
>> + }
>> +
>> + if (likely(lif_to_txqcq(lif, queue_index)))
>> + q = lif_to_txq(lif, queue_index);
>> + else
>> + q = lif_to_txq(lif, 0);
>> +
>> + ndescs = ionic_tx_descs_needed(q, skb);
>> + if (ndescs < 0)
>> + goto err_out_drop;
>> +
>> + if (!ionic_q_has_space(q, ndescs)) {
>> + netif_stop_subqueue(netdev, queue_index);
>> + q->stop++;
>> +
>> + /* Might race with ionic_tx_clean, check again */
>> + smp_rmb();
>> + if (ionic_q_has_space(q, ndescs)) {
>> + netif_wake_subqueue(netdev, queue_index);
>> + q->wake++;
>> + } else {
>> + return NETDEV_TX_BUSY;
> This should never really happen..
Couldn't we have an Rx interrupt and a call to ionic_tx_clean() in the
middle of this?
>
>> + }
>> + }
>> +
>> + if (skb_is_gso(skb))
>> + err = ionic_tx_tso(q, skb);
>> + else
>> + err = ionic_tx(q, skb);
>> +
>> + if (err)
>> + goto err_out_drop;
> .. at this point if you can't guarantee fitting biggest possible frame
> in, you have to stop the ring.
Yep, that would work.
Thanks,
sln
^ permalink raw reply
* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Karsten Wiborg @ 2019-07-01 18:15 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <20190701133507.GB25795@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 741 bytes --]
Hi Andrew, Heiner,
the device is a really small notebook. So detaching mains still leaves
the battery which is delicately built in. So can't currently remove
power completely.
Anyway can I deliver more debugging data to you guys which might add
support for the r8169 for this device?
Regards,
Karsten
On 01/07/2019 15:35, Andrew Lunn wrote:
>> When the vendor driver assigns a random MAC address, it writes it to the
>> chip. The related registers may be persistent (can't say exactly due to
>> missing documentation).
>
> If the device supports WOL, it could be it is powered using the
> standby supply, not the main supply. Try pulling the plug from the
> wall to really remove all power.
>
> Andrew
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4156 bytes --]
^ permalink raw reply
* [PATCH rdma-next 2/2] IB/mlx5: Implement VHCA tunnel mechanism in DEVX
From: Leon Romanovsky @ 2019-07-01 18:14 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Max Gurtovoy, Yishai Hadas,
Saeed Mahameed, linux-netdev
In-Reply-To: <20190701181402.25286-1-leon@kernel.org>
From: Max Gurtovoy <maxg@mellanox.com>
This mechanism will allow function-A to perform operations "on behalf"
of function-B via tunnel object. Function-A will have privileges for
creating and using this tunnel object.
For example, in the device emulation feature presented in Bluefield-1 SoC,
using device emulation capability, one can present NVMe function to the host OS.
Since the NVMe function doesn't have a normal command interface to the HCA HW,
here is a need to create a channel that will be able to issue commands "on behalf"
of this function.
This channel is the VHCA_TUNNEL general object. The emulation software will create
this tunnel for every managed function and issue commands via devx general cmd
interface using the appropriate tunnel ID. When devX context will receive a command
with non-zero vhca_tunnel_id, it will pass the command as-is down to the HCA.
All the validation, security and resource tracking of the commands and the created
tunneled objects is in the responsibility of the HCA FW. When a VHCA_TUNNEL object
destroyed, the device will issue an internal FLR (function level reset) to the
emulated function associated with this tunnel. This will destroy all the created
resources using the tunnel mechanism.
Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/hw/mlx5/devx.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index cb4f0fc79176..26ba6bf5d19a 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -894,12 +894,16 @@ static int devx_get_uid(struct mlx5_ib_ucontext *c, void *cmd_in)
return c->devx_uid;
}
-static bool devx_is_general_cmd(void *in)
+
+static bool devx_is_general_cmd(void *in, struct mlx5_ib_dev *dev)
{
u16 opcode = MLX5_GET(general_obj_in_cmd_hdr, in, opcode);
- if (opcode >= MLX5_CMD_OP_GENERAL_START &&
- opcode < MLX5_CMD_OP_GENERAL_END)
+ /* Pass all cmds for vhca_tunnel as general, tracking is done in FW */
+ if ((MLX5_CAP_GEN_64(dev->mdev, vhca_tunnel_commands) &&
+ MLX5_GET(general_obj_in_cmd_hdr, in, vhca_tunnel_id)) ||
+ (opcode >= MLX5_CMD_OP_GENERAL_START &&
+ opcode < MLX5_CMD_OP_GENERAL_END))
return true;
switch (opcode) {
@@ -1025,7 +1029,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OTHER)(
return uid;
/* Only white list of some general HCA commands are allowed for this method. */
- if (!devx_is_general_cmd(cmd_in))
+ if (!devx_is_general_cmd(cmd_in, dev))
return -EINVAL;
cmd_out = uverbs_zalloc(attrs, cmd_out_len);
@@ -1420,6 +1424,9 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
u32 obj_id;
u16 opcode;
+ if (MLX5_GET(general_obj_in_cmd_hdr, cmd_in, vhca_tunnel_id))
+ return -EINVAL;
+
uid = devx_get_uid(c, cmd_in);
if (uid < 0)
return uid;
@@ -1519,6 +1526,9 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
int err;
int uid;
+ if (MLX5_GET(general_obj_in_cmd_hdr, cmd_in, vhca_tunnel_id))
+ return -EINVAL;
+
uid = devx_get_uid(c, cmd_in);
if (uid < 0)
return uid;
@@ -1561,6 +1571,9 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_QUERY)(
int uid;
struct mlx5_ib_dev *mdev = to_mdev(c->ibucontext.device);
+ if (MLX5_GET(general_obj_in_cmd_hdr, cmd_in, vhca_tunnel_id))
+ return -EINVAL;
+
uid = devx_get_uid(c, cmd_in);
if (uid < 0)
return uid;
@@ -1698,6 +1711,9 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_ASYNC_QUERY)(
struct devx_async_cmd_event_file *ev_file;
struct devx_async_data *async_data;
+ if (MLX5_GET(general_obj_in_cmd_hdr, cmd_in, vhca_tunnel_id))
+ return -EINVAL;
+
uid = devx_get_uid(c, cmd_in);
if (uid < 0)
return uid;
--
2.20.1
^ permalink raw reply related
* [PATCH mlx5-next 1/2] net/mlx5: Introduce VHCA tunnel device capability
From: Leon Romanovsky @ 2019-07-01 18:14 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Max Gurtovoy, Yishai Hadas,
Saeed Mahameed, linux-netdev
In-Reply-To: <20190701181402.25286-1-leon@kernel.org>
From: Max Gurtovoy <maxg@mellanox.com>
When using the device emulation feature (introduced in Bluefield-1 SOC),
a privileged function (the device emulation manager) will be able to
create a channel to execute commands on behalf of the emulated function.
This channel will be a general object of type VHCA_TUNNEL that will have
a unique ID for each emulated function. This ID will be passed in each
cmd that will be issued by the emulation SW in a well known offset in
the command header.
This channel is needed since the emulated function doesn't have a normal
command interface to the HCA HW, but some basic configuration for that
function is needed (e.g. initialize and enable the HCA). For that matter,
a specific command-set was defined and only those commands will be issued
by the HCA.
Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
include/linux/mlx5/mlx5_ifc.h | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 5d2bf91e130b..a35e545a8de0 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -1329,7 +1329,13 @@ struct mlx5_ifc_cmd_hca_cap_bits {
u8 reserved_at_6c0[0x4];
u8 flex_parser_id_geneve_tlv_option_0[0x4];
- u8 reserved_at_6c8[0x138];
+ u8 reserved_at_6c8[0x18];
+
+ u8 reserved_at_6e0[0xa0];
+
+ u8 vhca_tunnel_commands[0x40];
+
+ u8 reserved_at_7c0[0x40];
};
enum mlx5_flow_destination_type {
@@ -9560,7 +9566,7 @@ struct mlx5_ifc_general_obj_in_cmd_hdr_bits {
u8 opcode[0x10];
u8 uid[0x10];
- u8 reserved_at_20[0x10];
+ u8 vhca_tunnel_id[0x10];
u8 obj_type[0x10];
u8 obj_id[0x20];
--
2.20.1
^ permalink raw reply related
* [PATCH rdma-next 0/2] DEVX VHCA tunnel support
From: Leon Romanovsky @ 2019-07-01 18:14 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Max Gurtovoy, Yishai Hadas,
Saeed Mahameed, linux-netdev
From: Leon Romanovsky <leonro@mellanox.com>
Hi,
Those two patches introduce VHCA tunnel mechanism to DEVX interface
needed for Bluefield SOC. See extensive commit messages for more
information.
Thanks
Max Gurtovoy (2):
net/mlx5: Introduce VHCA tunnel device capability
IB/mlx5: Implement VHCA tunnel mechanism in DEVX
drivers/infiniband/hw/mlx5/devx.c | 24 ++++++++++++++++++++----
include/linux/mlx5/mlx5_ifc.h | 10 ++++++++--
2 files changed, 28 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH v5 net-next 6/6] net: ethernet: ti: cpsw: add XDP support
From: Ilias Apalodimas @ 2019-07-01 18:09 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Ivan Khoronzhuk, grygorii.strashko, davem, ast, linux-kernel,
linux-omap, netdev, daniel, jakub.kicinski, john.fastabend
In-Reply-To: <20190701181901.150c0b71@carbon>
On Mon, Jul 01, 2019 at 06:19:01PM +0200, Jesper Dangaard Brouer wrote:
> On Sun, 30 Jun 2019 20:23:48 +0300
> Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
> > +static int cpsw_ndev_create_xdp_rxq(struct cpsw_priv *priv, int ch)
> > +{
> > + struct cpsw_common *cpsw = priv->cpsw;
> > + int ret, new_pool = false;
> > + struct xdp_rxq_info *rxq;
> > +
> > + rxq = &priv->xdp_rxq[ch];
> > +
> > + ret = xdp_rxq_info_reg(rxq, priv->ndev, ch);
> > + if (ret)
> > + return ret;
> > +
> > + if (!cpsw->page_pool[ch]) {
> > + ret = cpsw_create_rx_pool(cpsw, ch);
> > + if (ret)
> > + goto err_rxq;
> > +
> > + new_pool = true;
> > + }
> > +
> > + ret = xdp_rxq_info_reg_mem_model(rxq, MEM_TYPE_PAGE_POOL,
> > + cpsw->page_pool[ch]);
> > + if (!ret)
> > + return 0;
> > +
> > + if (new_pool) {
> > + page_pool_free(cpsw->page_pool[ch]);
> > + cpsw->page_pool[ch] = NULL;
> > + }
> > +
> > +err_rxq:
> > + xdp_rxq_info_unreg(rxq);
> > + return ret;
> > +}
>
> Looking at this, and Ilias'es XDP-netsec error handling path, it might
> be a mistake that I removed page_pool_destroy() and instead put the
> responsibility on xdp_rxq_info_unreg().
>
> As here, we have to detect if page_pool_create() was a success, and then
> if xdp_rxq_info_reg_mem_model() was a failure, explicitly call
> page_pool_free() because the xdp_rxq_info_unreg() call cannot "free"
> the page_pool object given it was not registered.
>
> Ivan's patch in[1], might be a better approach, which forced all
> drivers to explicitly call page_pool_free(), even-though it just
> dec-refcnt and the real call to page_pool_free() happened via
> xdp_rxq_info_unreg().
We did discuss that xdp_XXXXX naming might be confusing.
That being said since Ivan's approach serves 'special' hardware and fixes the
naming irregularity, i perfectly fine doing that as long as we clearly document
that the API is supposed to serve a pool per queue (unless the hardware needs to
deal with it differently)
>
> To better handle error path, I would re-introduce page_pool_destroy(),
> as a driver API, that would gracefully handle NULL-pointer case, and
> then call page_pool_free() with the atomic_dec_and_test(). (It should
> hopefully simplify the error handling code a bit)
>
> [1] https://lore.kernel.org/netdev/20190625175948.24771-2-ivan.khoronzhuk@linaro.org/
>
>
Thanks
/Ilias
> > +void cpsw_ndev_destroy_xdp_rxqs(struct cpsw_priv *priv)
> > +{
> > + struct cpsw_common *cpsw = priv->cpsw;
> > + struct xdp_rxq_info *rxq;
> > + int i;
> > +
> > + for (i = 0; i < cpsw->rx_ch_num; i++) {
> > + rxq = &priv->xdp_rxq[i];
> > + if (xdp_rxq_info_is_reg(rxq))
> > + xdp_rxq_info_unreg(rxq);
> > + }
> > +}
>
> Are you sure you need to test xdp_rxq_info_is_reg() here?
>
> You should just call xdp_rxq_info_unreg(rxq), if you know that this rxq
> should be registered. If your assumption failed, you will get a
> WARNing, and discover your driver level bug. This is one of the ways
> the API is designed to "detect" misuse of the API. (I found this
> rather useful, when I converted the approx 12 drivers using this
> xdp_rxq_info API).
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 net-next 16/19] ionic: Add driver stats
From: Shannon Nelson @ 2019-07-01 18:06 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629115324.7adfc3c9@cakuba.netronome.com>
On 6/29/19 11:53 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:31 -0700, Shannon Nelson wrote:
>> Add in the detailed statistics for ethtool -S that the driver
>> keeps as it processes packets. Display of the additional
>> debug statistics can be enabled through the ethtool priv-flags
>> feature.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>
>> +static void ionic_get_strings(struct net_device *netdev,
>> + u32 sset, u8 *buf)
>> +{
>> + struct lif *lif = netdev_priv(netdev);
>> +
>> + switch (sset) {
>> + case ETH_SS_STATS:
>> + ionic_get_stats_strings(lif, buf);
>> + break;
>> + case ETH_SS_PRIV_FLAGS:
>> + memcpy(buf, ionic_priv_flags_strings,
>> + PRIV_FLAGS_COUNT * ETH_GSTRING_LEN);
>> + break;
>> + case ETH_SS_TEST:
>> + // IONIC_TODO
>> + default:
>> + netdev_err(netdev, "Invalid sset %d\n", sset);
> Not really an error, as long as sset_count() returns a 0 nothing will
> happen. Also you can drop the SS_TEST if you don't report it.
Sure.
>> +
>> +#endif // _IONIC_STATS_H_
> Perhaps worth grepping the driver for C++ style comments?
Those nasty little things sneak in there when you're not looking, and
you try to get rid of them all, and there always seems to be one more
:-). Yes, I'll look again.
sln
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Matteo Croce @ 2019-07-01 18:03 UTC (permalink / raw)
To: Josh Elsasser
Cc: Greg Kroah-Hartman, Sasha Levin, stable, netdev, LKML,
David Miller
In-Reply-To: <20190701175241.GB9081@kroah.com>
On Mon, Jul 1, 2019 at 7:53 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jun 29, 2019 at 09:39:39PM +0200, Matteo Croce wrote:
> > On Sat, Jun 29, 2019 at 9:45 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Jun 28, 2019 at 07:03:01PM -0700, Josh Elsasser wrote:
> > > > On Jun 28, 2019, at 3:55 PM, Sasha Levin <sashal@kernel.org> wrote:
> > > >
> > > > > What's the upstream commit id?
> > > >
> > > > The commit wasn't needed upstream, as I only sent the original patch after
> > > > 79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") had
> > > > made the fix unnecessary in Linus' tree.
> > > >
> > > > May've gotten lost in the shuffle due to my poor Fixes tags. The patch in
> > > > question applied only on top of the 4.9 stable release at the time, but the
> > > > actual NPE had been around in some form since 3.11 / 0602129286705 ("net: add
> > > > low latency socket poll").
> > >
> > > Ok, can people then resend this and be very explicit as to why this is
> > > needed only in a stable kernel tree and get reviews from people agreeing
> > > that this really is the correct fix?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Hi Greg,
> >
> > I think that David alredy reviewed the patch here:
> >
> > https://lore.kernel.org/netdev/20180313.105115.682846171057663636.davem@davemloft.net/
> >
> > Anyway, I tested the patch and it fixes the panic, at least on my
> > iwlwifi card, so:
> >
> > Tested-by: Matteo Croce <mcroce@redhat.com>
>
> Ok, but what can I do with this? I need a real patch, in mail form,
> that I can apply. Not a web link to an email archive.
>
> You have read the stable kernel rules, right? :)
>
> greg k-h
Understood.
Josh, as you are the original author, can you please resend it to -stable?
Feel free to add this tag:
Tested-by: Matteo Croce <mcroce@redhat.com>
Regards,
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH v2 net-next 17/19] ionic: Add RSS support
From: Shannon Nelson @ 2019-07-01 18:03 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629114839.6cf1f048@cakuba.netronome.com>
On 6/29/19 11:48 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:32 -0700, Shannon Nelson wrote:
>> @@ -1260,10 +1266,24 @@ static struct lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index)
>> if (err)
>> goto err_out_free_lif_info;
>>
>> + /* allocate rss indirection table */
>> + tbl_sz = le16_to_cpu(lif->ionic->ident.lif.eth.rss_ind_tbl_sz);
>> + lif->rss_ind_tbl_sz = sizeof(*lif->rss_ind_tbl) * tbl_sz;
>> + lif->rss_ind_tbl = dma_alloc_coherent(dev, lif->rss_ind_tbl_sz,
>> + &lif->rss_ind_tbl_pa,
>> + GFP_KERNEL);
>> +
>> + if (!lif->rss_ind_tbl) {
>> + dev_err(dev, "Failed to allocate rss indirection table, aborting\n");
>> + goto err_out_free_qcqs;
>> + }
>> +
>> list_add_tail(&lif->list, &ionic->lifs);
>>
>> return lif;
>>
>> +err_out_free_qcqs:
>> + ionic_qcqs_free(lif);
>> err_out_free_lif_info:
>> dma_free_coherent(dev, lif->info_sz, lif->info, lif->info_pa);
>> lif->info = NULL;
>> @@ -1302,6 +1322,14 @@ static void ionic_lif_free(struct lif *lif)
>> {
>> struct device *dev = lif->ionic->dev;
>>
>> + /* free rss indirection table */
>> + if (lif->rss_ind_tbl) {
>> + dma_free_coherent(dev, lif->rss_ind_tbl_sz, lif->rss_ind_tbl,
>> + lif->rss_ind_tbl_pa);
> dma_free_coherent() should be able to deal with NULLs just fine.
> Besides you fail hard if the allocation failed, no?
I like checking my pointers, but sure, this can be simplified.
sln
>
>> + lif->rss_ind_tbl = NULL;
>> + lif->rss_ind_tbl_pa = 0;
>> + }
^ permalink raw reply
* Re: [PATCH 2/2] samples: pktgen: allow to specify destination port
From: David Miller @ 2019-07-01 18:02 UTC (permalink / raw)
To: danieltimlee; +Cc: brouer, netdev
In-Reply-To: <20190629133358.8251-2-danieltimlee@gmail.com>
From: "Daniel T. Lee" <danieltimlee@gmail.com>
Date: Sat, 29 Jun 2019 22:33:58 +0900
> Currently, kernel pktgen has the feature to specify udp destination port
> for sending packet. (e.g. pgset "udp_dst_min 9")
>
> But on samples, each of the scripts doesn't have any option to achieve this.
>
> This commit adds the DST_PORT option to specify the target port(s) in the script.
>
> -p : ($DST_PORT) destination PORT range (e.g. 433-444) is also allowed
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] samples: pktgen: add some helper functions for port parsing
From: David Miller @ 2019-07-01 18:02 UTC (permalink / raw)
To: danieltimlee; +Cc: brouer, netdev
In-Reply-To: <20190629133358.8251-1-danieltimlee@gmail.com>
From: "Daniel T. Lee" <danieltimlee@gmail.com>
Date: Sat, 29 Jun 2019 22:33:57 +0900
> This commit adds port parsing and port validate helper function to parse
> single or range of port(s) from a given string. (e.g. 1234, 443-444)
>
> Helpers will be used in prior to set target port(s) in samples/pktgen.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Applied.
^ permalink raw reply
* Re: 答复: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Pablo Neira Ayuso @ 2019-07-01 18:01 UTC (permalink / raw)
To: linmiaohe
Cc: David Ahern, kadlec@blackhole.kfki.hu, fw@strlen.de,
davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Mingfangsen
In-Reply-To: <cef929f9a14f462f9f7d3fa475f84e76@huawei.com>
On Sat, Jun 29, 2019 at 02:13:59PM +0000, linmiaohe wrote:
> On 6/29/19 20:20 PM, David Ahern wrote:
> > On 6/28/19 8:13 PM, linmiaohe wrote:
> > > You're right. Fib rules code would set FLOWI_FLAG_SKIP_NH_OIF flag.
> > > But I set it here for distinguish with the flags & XT_RPFILTER_LOOSE
> > > branch. Without this, they do the same work and maybe should be
> > > combined. I don't want to do that as that makes code confusing.
> > > Is this code snipet below ok ? If so, I would delete this flag setting.
> > >
> > > } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
> > > fl6.flowi6_oif = dev->ifindex;
> > > } else if ((flags & XT_RPFILTER_LOOSE) == 0)
> > > fl6.flowi6_oif = dev->ifindex;
>
> > that looks fine to me, but it is up to Pablo.
>
> @David Ahern Many thanks for your valuable advice.
>
> @ Pablo Hi, could you please tell me if this code snipet is ok?
> If not, which code would you prefer? It's very nice of you to
> figure it out for me. Thanks a lot.
Probably this?
} else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev) ||
(flags & XT_RPFILTER_LOOSE) == 0) {
fl6.flowi6_oif = dev->ifindex;
}
Thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 04/19] ionic: Add basic lif support
From: Shannon Nelson @ 2019-07-01 18:00 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629114526.4da68321@cakuba.netronome.com>
On 6/29/19 11:45 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:19 -0700, Shannon Nelson wrote:
>> @@ -64,4 +66,49 @@ int ionic_debugfs_add_ident(struct ionic *ionic)
>> ionic, &identity_fops) ? 0 : -ENOTSUPP;
>> }
>>
>> +int ionic_debugfs_add_sizes(struct ionic *ionic)
>> +{
>> + debugfs_create_u32("nlifs", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.dev.nlifs);
>> + debugfs_create_u32("nintrs", 0400, ionic->dentry, &ionic->nintrs);
>> +
>> + debugfs_create_u32("ntxqs_per_lif", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_TXQ]);
>> + debugfs_create_u32("nrxqs_per_lif", 0400, ionic->dentry,
>> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_RXQ]);
> Are these __le32s? Does the driver build cleanly with W=1 C=1?
Aside from a couple of "expression using sizeof(void)" messages that I
haven't been able to fully clean out, yes, the driver builds pretty
clean with sparse. In this particular case, I think the typecast stops
the complaint that you are expecting here. But yes, these would be
better off with le32_to_cpu().
sln
>
>> + return 0;
>> +}
^ permalink raw reply
* Re: [PATCH] net:gue.h:Fix shifting signed 32-bit value by 31 bits problem
From: David Miller @ 2019-07-01 18:00 UTC (permalink / raw)
To: bnvandana; +Cc: netdev, linux-kernel, skhan, gregkh, linux-kernel-mentees
In-Reply-To: <20190701141610.3681-1-bnvandana@gmail.com>
From: Vandana BN <bnvandana@gmail.com>
Date: Mon, 1 Jul 2019 19:46:10 +0530
> Fix GUE_PFLAG_REMCSUM to use "U" cast to avoid shifting signed
> 32-bit value by 31 bits problem.
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: icmp: allow flowlabel reflection in echo replies
From: David Miller @ 2019-07-01 17:59 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20190701133936.15238-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 1 Jul 2019 06:39:36 -0700
> Extend flowlabel_reflect bitmask to allow conditional
> reflection of incoming flowlabels in echo replies.
>
> Note this has precedence against auto flowlabels.
>
> Add flowlabel_reflect enum to replace hard coded
> values.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Greg Kroah-Hartman @ 2019-07-01 17:52 UTC (permalink / raw)
To: Matteo Croce
Cc: Josh Elsasser, Sasha Levin, stable, netdev, LKML, David Miller
In-Reply-To: <CAGnkfhzmGbeQe7L55nEv575XyubWqCLz=7NQPpH+TajDkkDiXg@mail.gmail.com>
On Sat, Jun 29, 2019 at 09:39:39PM +0200, Matteo Croce wrote:
> On Sat, Jun 29, 2019 at 9:45 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Jun 28, 2019 at 07:03:01PM -0700, Josh Elsasser wrote:
> > > On Jun 28, 2019, at 3:55 PM, Sasha Levin <sashal@kernel.org> wrote:
> > >
> > > > What's the upstream commit id?
> > >
> > > The commit wasn't needed upstream, as I only sent the original patch after
> > > 79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") had
> > > made the fix unnecessary in Linus' tree.
> > >
> > > May've gotten lost in the shuffle due to my poor Fixes tags. The patch in
> > > question applied only on top of the 4.9 stable release at the time, but the
> > > actual NPE had been around in some form since 3.11 / 0602129286705 ("net: add
> > > low latency socket poll").
> >
> > Ok, can people then resend this and be very explicit as to why this is
> > needed only in a stable kernel tree and get reviews from people agreeing
> > that this really is the correct fix?
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg,
>
> I think that David alredy reviewed the patch here:
>
> https://lore.kernel.org/netdev/20180313.105115.682846171057663636.davem@davemloft.net/
>
> Anyway, I tested the patch and it fixes the panic, at least on my
> iwlwifi card, so:
>
> Tested-by: Matteo Croce <mcroce@redhat.com>
Ok, but what can I do with this? I need a real patch, in mail form,
that I can apply. Not a web link to an email archive.
You have read the stable kernel rules, right? :)
greg k-h
^ permalink raw reply
* Re: net: check before dereferencing netdev_ops during busy poll
From: Greg Kroah-Hartman @ 2019-07-01 17:52 UTC (permalink / raw)
To: Matteo Croce; +Cc: stable, Sasha Levin, Josh Elsasser, netdev, LKML
In-Reply-To: <CAGnkfhxxw9keiNj_Qm=2GBYpY38HAq28cOROMRqXfbqq8wNbWQ@mail.gmail.com>
On Fri, Jun 28, 2019 at 06:34:58PM +0200, Matteo Croce wrote:
> Hi,
>
> Is there any reason for this panic fix not being applied in stable?
>
> https://lore.kernel.org/netdev/20180313053248.13654-1-jelsasser@appneta.com/T/
I can't apply patches from random urls :)
^ 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