* RE: [PATCH net 2/4] tls: Fix write space handling
From: Vakul Garg @ 2019-02-26 12:49 UTC (permalink / raw)
To: Boris Pismenny, aviadye@mellanox.com, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: eranbe@mellanox.com
In-Reply-To: <20190226121235.20784-3-borisp@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Tuesday, February 26, 2019 5:43 PM
> To: aviadye@mellanox.com; davejwatson@fb.com;
> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
> <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: eranbe@mellanox.com; borisp@mellanox.com
> Subject: [PATCH net 2/4] tls: Fix write space handling
>
> TLS device cannot use the sw context. This patch returns the original
> tls device write space handler and moves the sw/device specific portions
> to the relevant files.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records
> for performance")
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> ---
> include/net/tls.h | 3 +++
> net/tls/tls_device.c | 16 ++++++++++++++++
> net/tls/tls_main.c | 17 +++++++++--------
> net/tls/tls_sw.c | 15 +++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index a528a082da73..9d7c53737b13 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock
> *sk)
> return !!tls_sw_ctx_tx(ctx);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
> +
> static inline struct tls_offload_context_rx *
> tls_offload_ctx_rx(const struct tls_context *tls_ctx)
> {
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index 3e5e8e021a87..e8988b3f3236 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct
> sock *sk, int flags)
> return tls_push_data(sk, &msg_iter, 0, flags,
> TLS_RECORD_TYPE_DATA);
> }
>
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + int rc = 0;
> +
> + if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
> + gfp_t sk_allocation = sk->sk_allocation;
> +
> + sk->sk_allocation = GFP_ATOMIC;
> + rc = tls_push_partial_record(sk, ctx,
> + MSG_DONTWAIT |
> MSG_NOSIGNAL);
> + sk->sk_allocation = sk_allocation;
> + }
> +
> + return rc;
> +}
> +
> void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 7e05af75536d..11c1980a75cb 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct
> tls_context *ctx,
> static void tls_write_space(struct sock *sk)
> {
> struct tls_context *ctx = tls_get_ctx(sk);
> - struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> + int rc;
>
> /* If in_tcp_sendpages call lower protocol write space handler
> * to ensure we wake up any waiting operations there. For example
> @@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
> return;
> }
>
> - /* Schedule the transmission if tx list is ready */
> - if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> - /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx-
> >tx_bitmask))
> - schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> - }
> +#ifdef CONFIG_TLS_DEVICE
> + if (ctx->tx_conf == TLS_HW)
> + rc = tls_device_write_space(sk, ctx);
> + else
> +#endif
> + rc = tls_sw_write_space(sk, ctx);
>
> - ctx->sk_write_space(sk);
> + if (!rc)
Why do we need to check 'rc'?
If it is required, then ' ctx->sk_write_space(sk)' can move to tls_device_write_space()
since tls_sw_write_space() always returns '0'.
> + ctx->sk_write_space(sk);
> }
>
> static void tls_ctx_free(struct tls_context *ctx)
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 1cc830582fa8..4afa67b00aaf 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct
> *work)
> release_sock(sk);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> +
> + /* Schedule the transmission if tx list is ready */
> + if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> + /* Schedule the transmission */
> + if (!test_and_set_bit(BIT_TX_SCHEDULED,
> + &tx_ctx->tx_bitmask))
> + schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> + }
> +
> + return 0;
> +}
> +
> int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> --
> 2.12.2
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Sheng Lan @ 2019-02-26 13:02 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <20190225080147.30128f73@shemminger-XPS-13-9360>
> On Mon, 25 Feb 2019 22:49:39 +0800
> Sheng Lan <lansheng@huawei.com> wrote:
>
>> From: Sheng Lan <lansheng@huawei.com>
>> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
>>
>> It can be reproduced by following steps:
>> 1. virtio_net NIC is configured with gso/tso on
>> 2. configure nginx as http server with an index file bigger than 1M bytes
>> 3. use tc netem to produce duplicate packets and delay:
>> tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
>> 4. continually curl the nginx http server to get index file on client
>> 5. BUG_ON is seen quickly
>>
>> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
>> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
>> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G W 5.0.0-rc6 #2
>> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
>> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
>> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
>> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
>> [10258690.372094] FS: 0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
>> [10258690.372094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
>> [10258690.372094] Call Trace:
>> [10258690.372094] <IRQ>
>> [10258690.372094] skb_to_sgvec+0x11/0x40
>> [10258690.372094] start_xmit+0x38c/0x520 [virtio_net]
>> [10258690.372094] dev_hard_start_xmit+0x9b/0x200
>> [10258690.372094] sch_direct_xmit+0xff/0x260
>> [10258690.372094] __qdisc_run+0x15e/0x4e0
>> [10258690.372094] net_tx_action+0x137/0x210
>> [10258690.372094] __do_softirq+0xd6/0x2a9
>> [10258690.372094] irq_exit+0xde/0xf0
>> [10258690.372094] smp_apic_timer_interrupt+0x74/0x140
>> [10258690.372094] apic_timer_interrupt+0xf/0x20
>> [10258690.372094] </IRQ>
>>
>> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
>> linear data size and nonlinear data size, thus BUG_ON triggered. The
>> bad skb's nonlinear data size is less than skb->data_len, because the
>> skb is cloned and a part of related cloned skb's nonlinear data is
>> split off.
>>
>> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
>> some time in qdisc. Due to the delay time, the original skb will be pushed
>> again later in __tcp_push_pending_frames when tcp receives new packets.
>> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
>> the original skb will be fragmented and the skb's nonlinear data will be
>> split off. The length of the skb cloned by netem will not be updated.
>> When we use virtio_net NIC, the duplicated cloned skb will be filled into
>> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
>>
>> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
>> the duplicated skb's nonlinear data is independent.
>>
>> Signed-off-by: Sheng Lan <lansheng@huawei.com>
>> Reported-by: Qin Ji <jiqin.ji@huawei.com>
>>
>> Fixes: 0afb51e7 ("netem: reinsert for duplication")
>
> This sounds like a bug in the other layers (either TCP or Virtio net)
> not handling a cloned skb properly.
>
I have traced the route of skb by printk, let me take an example to describe the problem to make it clearly:
Mss value equals to 1448. Limit value is the split size when tcp do tso_fragment, is depending on the size of the sending congestion window and mss value.
TCP layer transmit the index file to client, the original skb1 size is large:
...
tcp_write_xmit (skb1->data_len == 62264, limit == 2*mss == 2896)
tso_fragment (it needs to be fragmented by limit value)
skb_split (after split, skb1->data_len == 2896, skb_shinfo(skb1)->frags[0] == 2896, skb_shinfo(skb1)->nr_frags == 1)
...
netem_enqueue (netem construct a duplicate packet of skb1 by skb_clone)
skb2 = skb_clone(skb1) (skb1->data_len == skb2->data_len == 2896, skb1 and skb2 share the nonlinear data frags[0] == 2896)
waiting 30ms (skb1 and skb2 will be delayed in qdisc queue due to the netem delay configuration)
TCP layer receives new packets and trys to retransmit the skb1:
tcp_rcv_established
__tcp_push_pending_frames
tcp_write_xmit (skb1->data_len == 2896, cwnd size decreased or packets in flight increased, cause the limit decreased to 1*mss == 1448)
tso_fragment (limit value is less than skb1->data_len, skb1 will be fragmented again)
skb_split (the second time split, skb1 is cloned now and share nonlinear data with skb2.
skb1->data_len == 1448, frags[0] == 1448, a part of shared nonlinear data has been split off)
Now we can see:
skb1->data_len == 1448
skb2->data_len == 2896 (2896 is wrong value)
frags[0] == 1448
The route of skb2:
netem_enqueue (netem construct a duplicate packet, skb2 = skb_clone(skb1), put skb2 into queue)
waiting 30ms (delayed packet)
...
netem_dequeue (skb2->data_len == 2896, frags[0] == 1448)
sch_direct_xmit
dev_hard_start_xmit
xmit_one
start_xmit [virtio_net driver]
skb_to_sgvec (BUG_ON here )
The key is that skb be split by skb_split in tso_fragment again after netem clone it and share nonlinear data with another skb.
Processing of TCP seems OK, which push and fragment delayed packets in write queue. And virtio_net is the trigger of the BUG_ON.
So I replaced skb_clone with skb_copy in netem_enqueue, and the method worked. Currently, I have no better idea to fix it,
would you give me some inspiring advice ? If I am wrong, please correct me.
Thanks
^ permalink raw reply
* Re: [PATCH 2/2] ath9K: debugfs: Fix SPUR-DOWN field
From: Kalle Valo @ 2019-02-26 13:08 UTC (permalink / raw)
To: Andrea Greco
Cc: johannes.berg, Andrea Greco, QCA ath9k Development,
David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190221231257.6221-1-andrea.greco.gapmilano@gmail.com>
Andrea Greco <a.greco@4sigma.it> wrote:
> SPUR DOWN field returns spurup instead of spurdown.
>
> Signed-off-by: Andrea Greco <a.greco@4sigma.it>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
d0480d4326e2 ath9k: debugfs: Fix SPUR-DOWN field
--
https://patchwork.kernel.org/patch/10824837/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath9k: remove set but not used variable 'acq'
From: Kalle Valo @ 2019-02-26 13:09 UTC (permalink / raw)
To: YueHaibing
Cc: QCA ath9k Development, YueHaibing, linux-wireless, netdev,
kernel-janitors
In-Reply-To: <20190225033246.127410-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/ath/ath9k/recv.c: In function 'ath_rx_count_airtime':
> drivers/net/wireless/ath/ath9k/recv.c:1010:18: warning:
> variable 'acq' set but not used [-Wunused-but-set-variable]
>
> It's not used after 89cea7493a34 ("ath9k: Switch to mac80211 TXQ scheduling
> and airtime APIs"). Also remove related variables.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
03af21d6ba35 ath9k: remove set but not used variable 'acq'
--
https://patchwork.kernel.org/patch/10828153/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Julius Niedworok @ 2019-02-26 13:13 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: linux-wireless, ga58taw, David Hildenbrand, nc, David S. Miller,
Johannes Berg, Edward Cree, Jiri Pirko, Ido Schimmel,
Petr Machata, Kirill Tkhai, Alexander Duyck, Amritha Nambiar,
Li RongQing, netdev, linux-kernel
In-Reply-To: <c522f2e0-1bef-8679-45f7-707854918820@hartkopp.net>
Hi Oliver,
> On 26.02.2019 12:04, Oliver Hartkopp wrote:
>
> Hi Julius,
>
(..)
>
> The reason for IFF_ECHO was, that the data frame which is sent onto the wire (by one application) is not visible to all the other applications on the same (local) host. Therefore a successful transmission on the wire triggers the 'echo' of the sent content into the local host.
>
Thank you for the explanation - I can adjust the comment, if you like to.
> So what are you getting back after you enabled IFF_ECHO on your mac80211 device?
>
> Is it just a 'status' about a sent packet, or is it the packet ('full content') itself?
We are actually getting back the full content of the packet. So it matches the behaviour of the 'echo' in CAN.
>
> Regards,
> Oliver
>
Many thanks,
Julius
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Johannes Berg @ 2019-02-26 13:33 UTC (permalink / raw)
To: Julius Niedworok, Oliver Hartkopp
Cc: linux-wireless, ga58taw, David Hildenbrand, nc, David S. Miller,
Edward Cree, Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
linux-kernel
In-Reply-To: <6A213CBE-0B6A-466A-B721-E6A728D4888D@gmx.net>
On Tue, 2019-02-26 at 14:13 +0100, Julius Niedworok wrote:
>
> Thank you for the explanation - I can adjust the comment, if you like to.
>
> > So what are you getting back after you enabled IFF_ECHO on your mac80211 device?
> >
> > Is it just a 'status' about a sent packet, or is it the packet ('full content') itself?
>
> We are actually getting back the full content of the packet. So it
> matches the behaviour of the 'echo' in CAN.
I don't think it does, really.
In CAN, if I understand correctly, this is used for regular operation
interfaces, where you might want to run 'tcpdump', on wifi the
equivalent would be 'tcpdump -i wlan0'. This *already* implements full
visibility of outgoing and incoming frames.
Not sure how CAN even manages *not to*, but I don't really need to care
:-)
You're proposing to add this to the *monitor* interfaces and you really
should have made the flag conditional on that to make that clear.
However, even on monitor interfaces, you typically *already* see the
frames you transmitted there (as raw frames, which is the only thing you
can do).
What you're proposing is to use IFF_ECHO to show frames transmitted
through *other* interfaces on the monitor interface.
I don't think the IFF_ECHO semantics really match this.
Additionally, drivers are sort of free to ignore the REQ_TX_STATUS, or
we could in the future add ways of using the _noskb to feed back TX
status to the state machines where needed, so I'm not really sure I even
_want_ this to be set in stone in such an API.
Now, I can also see how this can be useful for debugging, but it feels
to me like this should be a driver (debug) option?
johannes
^ permalink raw reply
* Re: Yet another approach for implementing connection tracking offload
From: Yossi Kuperman @ 2019-02-26 13:35 UTC (permalink / raw)
To: Marcelo Leitner
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Or Gerlitz, Rony Efraim,
davem@davemloft.net, Paul Blakey, netdev@vger.kernel.org,
Oz Shlomo
In-Reply-To: <20190222222354.GV10660@localhost.localdomain>
On 23/02/2019 0:23, Marcelo Leitner wrote:
> On Mon, Feb 18, 2019 at 07:00:19PM +0000, Yossi Kuperman wrote:
>> Hello All,
>>
>> Following is a description of yet another possible approach to
>> implement connection tracking offload. We would like to hear your
>> opinion. There is the “native” way of implementing such an offload
>> by mirroring the software tables to hardware. This way seems
>> straightforward and simple, but real life is much more complicated
>> than that. Alternatively, we can merge the data-path flows
>> (separated by recirc_id) and offload a single flow to hardware.
>>
>> The general idea is quite simple. When OVS-daemon configures TC with
>> a filter that recirculate, the driver merely pretends to offload it
>> and return success. Upon packet arrival (in software) we let it
> This has potential to be a support nightmare: things that should and
> seems to be but are actually not and expected not to be.
> IOW, not_in_hw should still be there somehow.
Agree, we should handle this properly.
Please note we have a similar behavior with MT. From TC perspective it
may seem that everything is offloaded (in_hw), but we will encounter a
miss for every new connection.
>> traverse TC as usual, except for now we notify the driver on each
>> successful match. By doing this, the driver has all the necessary
>> information to merge the participating flows---including connection
>> tracking 5-tuple---into one equivalent flow. We do such a merge and
>> offload only if the connection is established. Note: the same
>> mechanism to communicate a 5-tuple to the driver can be used to
>> notify on a filter match.
>>
>> It is the driver responsibility to build and maintain the list of
>> filters a (specific) packet hit along the TC walk. Once we reach the
> I'm assuming this could be a shared code amongst the drivers, like
> DIM. We really don't want different algorithms for this.
> With that, we would be using the driver as just a temporary storage,
> for the matches/actions. Maybe we can do that with skb extensions?
>
> Like, turn on a flight recorder extention and have one especial last
> tc action (or even embed it into core tc) to process it when the
> traversing finishes. And only then call the driver to update whatever
> is needed.
>
It is a possibility.
>> last filter (a terminating one, e.g., forward) the driver posts a
>> work on a dedicated work-queue. In this work-queue context, we merge
>> the participating filters and create a new filter that is logically
>> equal (match + actions). The merge itself is not complicated as it
>> might seems—TC does all the heavy lifting, this is not a random list
>> of filters. At this point, we configure the hardware with one
>> filter, either we have a match and the packet is handled by the
>> hardware, or we don’t and the packet goes to software unmodified.
>>
>> Going along this path we must tackle two things: 1) counters and 2)
>> TC filter deletion. 1) We must maintain TC counters as the user
>> expect. Each merged filter holds a list of filters it is derived
>> from, parents. Once an update is available for a merged filter
>> counter, the driver must update the corresponding parents
>> appropriately. 2) Upon TC filer deletion it is mandatory to remove
>> all the derived (merged) filters from the hardware as consequence.
> I'm failing to see how this would address two features of CT:
>
> - window validation. How would the card know that it should perform
> window validation on these packets?
The driver should allocate a context and mapped it appropriately, similar
way one should do for the MT approach. It is possible that one context
is shared between two or more merged flows. Context holds TCP state
in hardware.
> - flow stats. If it offloads the result of a merge, how can it
> retrieve the specific stats for each 5-tuple? Even with the lists
> you mentioned, once the 5-tuples are aggregated, we can't separate
> them anymore.
Each merged flow maintains a list of flows it is derived from, including
any 5-tuple involved. We still have an entry for each 5-tuple in the driver,
and we keep it up-to-date based on statistics from related merged flows.
Upper layers can query the driver for the relevant information given a
5-tuple.
Am I missing something?
>>
>>
>> Pros & Cons
>>
>> Pros: 1) Circumvent the complexity involved with continuation in
>> software where the hardware left off. 2) Simplifies the hardware
>> pipeline with only one filter and might improve the overall
>> performance.
>>
>> Cons: 1) Only applicable to OVS-oriented filters, will not support
>> priorities and overlapping filters. 2) Merger logic might consume
>> CPU cycles which might impact the rate of filters we can offload.
>> However, this overhead is believed to be negligible, if implemented
>> carefully. 3) Requires TC/flower to notify the driver on each filter
>> match (that is the only change needed above the driver).
> On more general comments, I have the feeling that this would be best
> done in OvS:
> - it has 2 flow "translations": one from OpenFlow to OvS/tc sw
> datapath, and then OvS/tc sw to hw. If the resulting flows are
> better for the hw, aren't them better for sw datapath too?
A valid point. I'm not sure by how much it will improve the slow-path
performance, but it will surely complicate the code. Doing this "trick"
definitely improves the hardware performance.
> - if this is for OvS-oriented filters, why can't/shouldn't OvS do this
> processing instead?
>
It is something we have considered before, as it makes sense. However,
going this way means for every new connection (different 5-tuple) a costly
"travel" to OVS user-space.
> Somehow I keep thinking this is replacing conntrack (as in the
> conntrack subsystem) with ipset offloading.
>
> All in all, any approach is probably fine but I'm struggling to see a
> path forward from this to actual CT offloading. Maybe I missed
> something.
> Cheers,
> Marcelo
>
>>
>>
>> Both approaches share the same software model, most of the code
>> above the driver is shared. This approach can be considered
>> temporary until the hardware will mature.
>>
>> What do you think about this approach?
>>
>> If something is not clear please let me know and I will do my best
>> to clarify.
>>
>> Cheers, Kuperman
>>
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
From: Roi Dayan @ 2019-02-26 13:38 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-4-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch is a little improvement. Simplify the parse_tc_fdb_actions().
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 708f819..e6583b9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2475,13 +2475,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
>
> static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> struct flow_action *flow_action,
> - struct mlx5e_tc_flow_parse_attr *parse_attr,
> struct mlx5e_tc_flow *flow,
> struct netlink_ext_ack *extack)
> {
> struct pedit_headers_action hdrs[2] = {};
> struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
> struct mlx5_esw_flow_attr *attr = flow->esw_attr;
> + struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
> struct mlx5e_rep_priv *rpriv = priv->ppriv;
> const struct ip_tunnel_info *info = NULL;
> const struct flow_action_entry *act;
> @@ -2796,7 +2796,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
> if (err)
> goto err_free;
>
> - err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
> + err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
> if (err)
> goto err_free;
>
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* [PATCH net-next v4 2/4] arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the LS1028A RDB board
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
The LS1028A RDB board features an Atheros PHY connected over
SGMII to the ENETC PF0 (or Port0). ENETC Port1 (PF1) has no
external connection on this board, so it can be disabled for now.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2 - added a mdio node as parent for the phy node
v3 - none
v4 - none
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index fdeb417..f86b054 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -71,3 +71,20 @@
&duart1 {
status = "okay";
};
+
+&enetc_port0 {
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+};
+
+&enetc_port1 {
+ status = "disabled";
+};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 4/4] dt-bindings: net: freescale: enetc: Add connection bindings for ENETC ethernet nodes
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
Define connection bindings (external PHY connections and internal links)
for the ENETC on-chip ethernet controllers.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v3 - added this patch to the set
v4 - documented strictly the enetc node bindings, changed node type
from "pci" to "ethernet" and added compatible string (Rob H.)
.../devicetree/bindings/net/fsl-enetc.txt | 69 ++++++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/fsl-enetc.txt
diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
new file mode 100644
index 0000000..c812e25
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -0,0 +1,69 @@
+* ENETC ethernet device tree bindings
+
+Depending on board design and ENETC port type (internal or
+external) there are two supported link modes specified by
+below device tree bindings.
+
+Required properties:
+
+- reg : Specifies PCIe Device Number and Function
+ Number of the ENETC endpoint device, according
+ to parent node bindings.
+- compatible : Should be "fsl,enetc".
+
+1) The ENETC external port is connected to a MDIO configurable phy:
+
+In this case, the ENETC node should include a "mdio" sub-node
+that in turn should contain the "ethernet-phy" node describing the
+external phy. Below properties are required, their bindings
+already defined in ethernet.txt or phy.txt, under
+Documentation/devicetree/bindings/net/*.
+
+Required:
+
+- phy-handle : Phandle to a PHY on the MDIO bus.
+ Defined in ethernet.txt.
+
+- phy-connection-type : Defined in ethernet.txt.
+
+- mdio : "mdio" node, defined in mdio.txt.
+
+- ethernet-phy : "ethernet-phy" node, defined in phy.txt.
+
+Example:
+
+ ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+ };
+
+2) The ENETC port is an internal port or has a fixed-link external
+connection:
+
+In this case, the ENETC port node defines a fixed link connection,
+as specified by "fixed-link.txt", under
+Documentation/devicetree/bindings/net/*.
+
+Required:
+
+- fixed-link : "fixed-link" node, defined in "fixed-link.txt".
+
+Example:
+ ethernet@0,2 {
+ compatible = "fsl,enetc";
+ reg = <0x000200 0 0 0 0>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 3/4] enetc: Add ENETC PF level external MDIO support
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
Each ENETC PF has its own MDIO interface, the corresponding
MDIO registers are mapped in the ENETC's Port register block.
The current patch adds a driver for these PF level MDIO buses,
so that each PF can manage directly its own external link.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
v2 - used readx_poll_timeout()
- added mdio node child to the port node
- added code comments, removed redundant err messages,
minor code cleanup
v3 - replaced dev_err w/ dev_dbg for read() issues
v4 - none
drivers/net/ethernet/freescale/enetc/Makefile | 3 +-
drivers/net/ethernet/freescale/enetc/enetc_mdio.c | 199 ++++++++++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 12 ++
drivers/net/ethernet/freescale/enetc/enetc_pf.h | 6 +
4 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_mdio.c
diff --git a/drivers/net/ethernet/freescale/enetc/Makefile b/drivers/net/ethernet/freescale/enetc/Makefile
index 6976602..7139e41 100644
--- a/drivers/net/ethernet/freescale/enetc/Makefile
+++ b/drivers/net/ethernet/freescale/enetc/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_FSL_ENETC) += fsl-enetc.o
-fsl-enetc-$(CONFIG_FSL_ENETC) += enetc.o enetc_cbdr.o enetc_ethtool.o
+fsl-enetc-$(CONFIG_FSL_ENETC) += enetc.o enetc_cbdr.o enetc_ethtool.o \
+ enetc_mdio.o
fsl-enetc-$(CONFIG_PCI_IOV) += enetc_msg.o
fsl-enetc-objs := enetc_pf.o $(fsl-enetc-y)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
new file mode 100644
index 0000000..77b9cd1
--- /dev/null
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2019 NXP */
+
+#include <linux/mdio.h>
+#include <linux/of_mdio.h>
+#include <linux/iopoll.h>
+#include <linux/of.h>
+
+#include "enetc_pf.h"
+
+struct enetc_mdio_regs {
+ u32 mdio_cfg; /* MDIO configuration and status */
+ u32 mdio_ctl; /* MDIO control */
+ u32 mdio_data; /* MDIO data */
+ u32 mdio_addr; /* MDIO address */
+};
+
+#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
+
+#define ENETC_MDIO_REG_OFFSET 0x1c00
+#define ENETC_MDC_DIV 258
+
+#define MDIO_CFG_CLKDIV(x) ((((x) >> 1) & 0xff) << 8)
+#define MDIO_CFG_BSY BIT(0)
+#define MDIO_CFG_RD_ER BIT(1)
+#define MDIO_CFG_ENC45 BIT(6)
+ /* external MDIO only - driven on neg MDC edge */
+#define MDIO_CFG_NEG BIT(23)
+
+#define MDIO_CTL_DEV_ADDR(x) ((x) & 0x1f)
+#define MDIO_CTL_PORT_ADDR(x) (((x) & 0x1f) << 5)
+#define MDIO_CTL_READ BIT(15)
+#define MDIO_DATA(x) ((x) & 0xffff)
+
+#define TIMEOUT 1000
+static int enetc_mdio_wait_complete(struct enetc_mdio_regs __iomem *regs)
+{
+ u32 val;
+
+ return readx_poll_timeout(enetc_rd_reg, ®s->mdio_cfg, val,
+ !(val & MDIO_CFG_BSY), 10, 10 * TIMEOUT);
+}
+
+static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
+ u16 value)
+{
+ struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ u32 mdio_ctl, mdio_cfg;
+ u16 dev_addr;
+ int ret;
+
+ mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
+ if (regnum & MII_ADDR_C45) {
+ dev_addr = (regnum >> 16) & 0x1f;
+ mdio_cfg |= MDIO_CFG_ENC45;
+ } else {
+ /* clause 22 (ie 1G) */
+ dev_addr = regnum & 0x1f;
+ mdio_cfg &= ~MDIO_CFG_ENC45;
+ }
+
+ enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* set port and dev addr */
+ mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+
+ /* set the register address */
+ if (regnum & MII_ADDR_C45) {
+ enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+ }
+
+ /* write the value */
+ enetc_wr_reg(®s->mdio_data, MDIO_DATA(value));
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+ struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ u32 mdio_ctl, mdio_cfg;
+ u16 dev_addr, value;
+ int ret;
+
+ mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
+ if (regnum & MII_ADDR_C45) {
+ dev_addr = (regnum >> 16) & 0x1f;
+ mdio_cfg |= MDIO_CFG_ENC45;
+ } else {
+ dev_addr = regnum & 0x1f;
+ mdio_cfg &= ~MDIO_CFG_ENC45;
+ }
+
+ enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* set port and device addr */
+ mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+
+ /* set the register address */
+ if (regnum & MII_ADDR_C45) {
+ enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+ }
+
+ /* initiate the read */
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* return all Fs if nothing was there */
+ if (enetc_rd_reg(®s->mdio_cfg) & MDIO_CFG_RD_ER) {
+ dev_dbg(&bus->dev,
+ "Error while reading PHY%d reg at %d.%hhu\n",
+ phy_id, dev_addr, regnum);
+ return 0xffff;
+ }
+
+ value = enetc_rd_reg(®s->mdio_data) & 0xffff;
+
+ return value;
+}
+
+int enetc_mdio_probe(struct enetc_pf *pf)
+{
+ struct device *dev = &pf->si->pdev->dev;
+ struct enetc_mdio_regs __iomem *regs;
+ struct device_node *np;
+ struct mii_bus *bus;
+ int ret;
+
+ bus = mdiobus_alloc_size(sizeof(regs));
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = "Freescale ENETC MDIO Bus";
+ bus->read = enetc_mdio_read;
+ bus->write = enetc_mdio_write;
+ bus->parent = dev;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+ /* store the enetc mdio base address for this bus */
+ regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
+ bus->priv = regs;
+
+ np = of_get_child_by_name(dev->of_node, "mdio");
+ if (!np) {
+ dev_err(dev, "MDIO node missing\n");
+ ret = -EINVAL;
+ goto err_registration;
+ }
+
+ ret = of_mdiobus_register(bus, np);
+ if (ret) {
+ of_node_put(np);
+ dev_err(dev, "cannot register MDIO bus\n");
+ goto err_registration;
+ }
+
+ of_node_put(np);
+ pf->mdio = bus;
+
+ return 0;
+
+err_registration:
+ mdiobus_free(bus);
+
+ return ret;
+}
+
+void enetc_mdio_remove(struct enetc_pf *pf)
+{
+ if (pf->mdio) {
+ mdiobus_unregister(pf->mdio);
+ mdiobus_free(pf->mdio);
+ }
+}
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 7d28f5e..15876a6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -746,6 +746,7 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
{
+ struct enetc_pf *pf = enetc_si_priv(priv->si);
struct device_node *np = priv->dev->of_node;
int err;
@@ -770,12 +771,22 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
priv->phy_node = of_node_get(np);
}
+ if (!of_phy_is_fixed_link(np)) {
+ err = enetc_mdio_probe(pf);
+ if (err) {
+ of_node_put(priv->phy_node);
+ return err;
+ }
+ }
+
priv->if_mode = of_get_phy_mode(np);
if (priv->if_mode < 0) {
dev_err(priv->dev, "missing phy type\n");
of_node_put(priv->phy_node);
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
+ else
+ enetc_mdio_remove(pf);
return -EINVAL;
}
@@ -898,6 +909,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
unregister_netdev(si->ndev);
+ enetc_mdio_remove(pf);
enetc_of_put_phy(priv);
enetc_free_msix(priv);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 2061ae5..10dd1b5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -42,8 +42,14 @@ struct enetc_pf {
char vlan_promisc_simap; /* bitmap of SIs in VLAN promisc mode */
DECLARE_BITMAP(vlan_ht_filter, ENETC_VLAN_HT_SIZE);
DECLARE_BITMAP(active_vlans, VLAN_N_VID);
+
+ struct mii_bus *mdio; /* saved for cleanup */
};
int enetc_msg_psi_init(struct enetc_pf *pf);
void enetc_msg_psi_free(struct enetc_pf *pf);
void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int mbox_id, u16 *status);
+
+/* MDIO */
+int enetc_mdio_probe(struct enetc_pf *pf);
+void enetc_mdio_remove(struct enetc_pf *pf);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 1/4] arm64: dts: fsl: ls1028a: Add PCI IERC node and ENETC endpoints
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
The LS1028A SoC features a PCI Integrated Endpoint Root Complex
(IERC) defining several integrated PCI devices, including the ENETC
ethernet controller integrated endpoints (IEPs). The IERC implements
ECAM (Enhanced Configuration Access Mechanism) to provide access
to the PCIe config space of the IEPs. This means the the IEPs
(including ENETC) do not support the standard PCIe BARs, instead
the Enhanced Allocation (EA) capability structures in the ECAM space
are used to fix the base addresses in the system, and the PCI
subsystem uses these structures for device enumeration and discovery.
The "ranges" entries contain basic information from these EA capabily
structures required by the kernel for device enumeration.
The current patch also enables the first 2 ENETC PFs (Physiscal
Functions) and the associated VFs (Virtual Functions), 2 VFs for
each PF. Each of these ENETC PFs has an external ethernet port
on the LS1028A SoC.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2 - none
v3 - none
v4 - changed enetc node names from "pci" to "ethernet" and
added compatible string (as requested by Rob H.)
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 35 ++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index a8cf92a..2896bbc 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -335,5 +335,40 @@
<GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
+ compatible = "pci-host-ecam-generic";
+ reg = <0x01 0xf0000000 0x0 0x100000>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ msi-parent = <&its>;
+ device_type = "pci";
+ bus-range = <0x0 0x0>;
+ dma-coherent;
+ msi-map = <0 &its 0x17 0xe>;
+ iommu-map = <0 &smmu 0x17 0xe>;
+ /* PF0-6 BAR0 - non-prefetchable memory */
+ ranges = <0x82000000 0x0 0x00000000 0x1 0xf8000000 0x0 0x160000
+ /* PF0-6 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf8160000 0x0 0x070000
+ /* PF0: VF0-1 BAR0 - non-prefetchable memory */
+ 0x82000000 0x0 0x00000000 0x1 0xf81d0000 0x0 0x020000
+ /* PF0: VF0-1 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf81f0000 0x0 0x020000
+ /* PF1: VF0-1 BAR0 - non-prefetchable memory */
+ 0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000
+ /* PF1: VF0-1 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>;
+
+ enetc_port0: ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ };
+ enetc_port1: ethernet@0,1 {
+ compatible = "fsl,enetc";
+ reg = <0x000100 0 0 0 0>;
+ };
+ };
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 0/4] enetc: Add mdio support and device tree nodes
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
This is the missing part to enable PCI probing of the ENETC ethernet
ports on the LS1028A SoC and external traffic on the LS1028A RDB board.
It's one of the first items on the TODO list for the recently merged
ENETC ethernet driver.
v3: Add DT bindings doc for ENETC connections
v4: none
Claudiu Manoil (4):
arm64: dts: fsl: ls1028a: Add PCI IERC node and ENETC endpoints
arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the
LS1028A RDB board
enetc: Add ENETC PF level external MDIO support
dt-bindings: net: freescale: enetc: Add connection bindings for ENETC
ethernet nodes
.../devicetree/bindings/net/fsl-enetc.txt | 69 +++++++
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 17 ++
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 35 ++++
drivers/net/ethernet/freescale/enetc/Makefile | 3 +-
drivers/net/ethernet/freescale/enetc/enetc_mdio.c | 199 +++++++++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 12 ++
drivers/net/ethernet/freescale/enetc/enetc_pf.h | 6 +
7 files changed, 340 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/net/fsl-enetc.txt
create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_mdio.c
--
2.7.4
^ permalink raw reply
* Re: [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Boris Pismenny @ 2019-02-26 13:43 UTC (permalink / raw)
To: Vakul Garg, Aviad Yehezkel, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: Eran Ben Elisha
In-Reply-To: <DB7PR04MB4252FD5C491CA99AF45BD10C8B7B0@DB7PR04MB4252.eurprd04.prod.outlook.com>
On 2/26/2019 2:38 PM, Vakul Garg wrote:
>
>
>> -----Original Message-----
>> From: Boris Pismenny <borisp@mellanox.com>
>> Sent: Tuesday, February 26, 2019 5:43 PM
>> To: aviadye@mellanox.com; davejwatson@fb.com;
>> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
>> <vakul.garg@nxp.com>; netdev@vger.kernel.org
>> Cc: eranbe@mellanox.com; borisp@mellanox.com
>> Subject: [PATCH net 3/4] tls: Fix mixing between async capable and async
>>
>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>
>> Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
>> application data TLS records. Moreover, it assumes that if the cipher can be
>> handled asynchronously, then all packets will be processed asynchronously.
>>
>> However, this assumption is not always true.
>
> Could you please elaborate, what happens?
>
When decryption doesn't occur asynchronously e.g. return code is not
EINPROGRESS, then async should be turned off.
>> Specifically, for AES-GCM in
>> TLS1.2, it causes data corruption, and breaks user applications.
>>
>> This patch fixes this problem by separating the async capability from the
>> decryption operation result.
>>
>> Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data
>> records")
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> Reviewed-by: Boris Pismenny <borisp@mellanox.com>
>> ---
>> net/tls/tls_sw.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
>> 4afa67b00aaf..f515cd7e984e 100644
>> --- a/net/tls/tls_sw.c
>> +++ b/net/tls/tls_sw.c
>> @@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
>> bool zc = false;
>> int to_decrypt;
>> int chunk = 0;
>> - bool async;
>> + bool async_capable;
>> + bool async = false;
>>
>> skb = tls_wait_data(sk, psock, flags, timeo, &err);
>> if (!skb) {
>> @@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
>>
>> /* Do not use async mode if record is non-data */
>> if (ctx->control == TLS_RECORD_TYPE_DATA)
>> - async = ctx->async_capable;
>> + async_capable = ctx->async_capable;
>> else
>> - async = false;
>> + async_capable = false;
>>
>> err = decrypt_skb_update(sk, skb, &msg->msg_iter,
>> - &chunk, &zc, async);
>> + &chunk, &zc, async_capable);
>> if (err < 0 && err != -EINPROGRESS) {
>> tls_err_abort(sk, EBADMSG);
>> goto recv_end;
>> }
>>
>> - if (err == -EINPROGRESS)
>> + if (err == -EINPROGRESS && async_capable) {
> Why do we need to check 'async_capable'?
> Do we get err == -EINPROGRESS even when async_capable is false?
>
I've missed this. I'll remove this and send V2.
>> + async = true;
>> num_async++;
>> - else if (prot->version == TLS_1_3_VERSION)
>> + } else if (prot->version == TLS_1_3_VERSION) {
>> tlm->control = ctx->control;
>> + }
>>
>> /* If the type of records being processed is not known yet,
>> * set it to record type just dequeued. If it is already known,
>> --
>> 2.12.2
>
>
>
>
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
From: Roi Dayan @ 2019-02-26 13:43 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-5-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch deletes unnecessary setting of the esw_attr->parse_attr
> to parse_attr in parse_tc_fdb_actions() because it is already done
> by the mlx5e_flow_esw_attr_init() function.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index e6583b9..d9fcb14 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2566,7 +2566,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> out_dev->ifindex;
> parse_attr->tun_info[attr->out_count] = *info;
> encap = false;
> - attr->parse_attr = parse_attr;
> attr->dests[attr->out_count].flags |=
> MLX5_ESW_DEST_ENCAP;
> attr->out_count++;
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v2 5/5] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
From: Roi Dayan @ 2019-02-26 13:49 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-6-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The encapsulation is not supported for mlx5 VFs. When we try to
> offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
> This patch changes the returned value and ignore to confuse user.
>
> For example: (p2p1_0 is VF net device)
> tc filter add dev p2p1_0 protocol ip parent ffff: prio 1 flower skip_sw \
> src_mac e4:11:22:33:44:01 \
> action tunnel_key set \
> src_ip 1.1.1.100 \
> dst_ip 1.1.1.200 \
> dst_port 4789 id 100 \
> action mirred egress redirect dev vxlan0
>
> "RTNETLINK answers: Invalid argument"
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index d9fcb14..f5029ea 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2302,7 +2302,8 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv,
> }
> break;
> default:
> - return -EINVAL;
> + NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> + return -EOPNOTSUPP;
> }
> }
>
> @@ -2624,7 +2625,8 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> break;
> }
> default:
> - return -EINVAL;
> + NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> + return -EOPNOTSUPP;
> }
> }
>
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: Roi Dayan @ 2019-02-26 13:50 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-3-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> If we try to offload decapsulation actions to VFs hw, we get the log [1].
> It's not friendly, because the kind of net device is null, and we don't
> know what '0' means.
>
> [1] "mlx5_core 0000:05:01.2 vf_0: decapsulation offload is not supported for net device (0)"
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> index bdcc5e7..6cbfbfa 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
> @@ -84,7 +84,7 @@ static const char *mlx5e_netdev_kind(struct net_device *dev)
> if (dev->rtnl_link_ops)
> return dev->rtnl_link_ops->kind;
> else
> - return "";
> + return "unknown";
> }
>
> static int mlx5e_route_lookup_ipv6(struct mlx5e_priv *priv,
> @@ -620,8 +620,10 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
> headers_c, headers_v);
> } else {
> netdev_warn(priv->netdev,
> - "decapsulation offload is not supported for %s net device (%d)\n",
> - mlx5e_netdev_kind(filter_dev), tunnel_type);
> + "decapsulation offload is not supported for %s (kind: \"%s\")\n",
> + netdev_name(filter_dev),
> + mlx5e_netdev_kind(filter_dev));
> +
> return -EOPNOTSUPP;
> }
> return err;
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink
From: Petr Machata @ 2019-02-26 13:51 UTC (permalink / raw)
To: Roopa Prabhu
Cc: davem@davemloft.net, netdev@vger.kernel.org,
dsa@cumulusnetworks.com, sd@queasysnail.net,
johannes@sipsolutions.net
In-Reply-To: <1551160982-32685-2-git-send-email-roopa@cumulusnetworks.com>
Roopa Prabhu <roopa@cumulusnetworks.com> writes:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds extack coverage in vxlan link
> create and changelink paths. Introduces a new helper
> vxlan_nl2flags to consolidate flag attribute validation.
>
> thanks to Johannes Berg for some tips to construct the
> generic vxlan flag extack strings.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++-----------------
> include/net/vxlan.h | 31 ++++++++
> 2 files changed, 172 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 577201c..a3c46d7 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
> return err;
> }
>
> +/* Set/clear flags based on attribute */
> +static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
> + int attrtype, unsigned long mask, bool changelink,
> + bool changelink_supported,
> + struct netlink_ext_ack *extack)
> +{
> + unsigned long flags;
> +
> + if (!tb[attrtype])
> + return 0;
> +
> + if (changelink && !changelink_supported) {
> + vxlan_flag_attr_error(attrtype, extack);
> + return -EOPNOTSUPP;
> + }
> +
> + if (vxlan_policy[attrtype].type == NLA_FLAG)
> + flags = conf->flags | mask;
> + else if (nla_get_u8(tb[attrtype]))
> + flags = conf->flags | mask;
> + else
> + flags = conf->flags & ~mask;
Many of the flags for which you call this don't actually have the else
branch. However I suspect there are no good reasons not to allow
resetting a flag.
Reviewed-by: Petr Machata <petrm@mellanox.com>
> +
> + conf->flags = flags;
> +
> + return 0;
> +}
> +
> static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> struct net_device *dev, struct vxlan_config *conf,
> - bool changelink)
> + bool changelink, struct netlink_ext_ack *extack)
> {
> struct vxlan_dev *vxlan = netdev_priv(dev);
> + int err = 0;
>
> memset(conf, 0, sizeof(*conf));
>
> @@ -3598,40 +3627,54 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> if (data[IFLA_VXLAN_ID]) {
> __be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
>
> - if (changelink && (vni != conf->vni))
> + if (changelink && (vni != conf->vni)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID], "Cannot change VNI");
> return -EOPNOTSUPP;
> + }
> conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
> }
>
> if (data[IFLA_VXLAN_GROUP]) {
> - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET))
> + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP], "New group address family does not match old group");
> return -EOPNOTSUPP;
> + }
>
> conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]);
> conf->remote_ip.sa.sa_family = AF_INET;
> } else if (data[IFLA_VXLAN_GROUP6]) {
> - if (!IS_ENABLED(CONFIG_IPV6))
> + if (!IS_ENABLED(CONFIG_IPV6)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "IPv6 support not enabled in the kernel");
> return -EPFNOSUPPORT;
> + }
>
> - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6))
> + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "New group address family does not match old group");
> return -EOPNOTSUPP;
> + }
>
> conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]);
> conf->remote_ip.sa.sa_family = AF_INET6;
> }
>
> if (data[IFLA_VXLAN_LOCAL]) {
> - if (changelink && (conf->saddr.sa.sa_family != AF_INET))
> + if (changelink && (conf->saddr.sa.sa_family != AF_INET)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL], "New local address family does not match old");
> return -EOPNOTSUPP;
> + }
>
> conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]);
> conf->saddr.sa.sa_family = AF_INET;
> } else if (data[IFLA_VXLAN_LOCAL6]) {
> - if (!IS_ENABLED(CONFIG_IPV6))
> + if (!IS_ENABLED(CONFIG_IPV6)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "IPv6 support not enabled in the kernel");
> return -EPFNOSUPPORT;
> + }
>
> - if (changelink && (conf->saddr.sa.sa_family != AF_INET6))
> + if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "New local address family does not match old");
> return -EOPNOTSUPP;
> + }
>
> /* TODO: respect scope id */
> conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
> @@ -3648,9 +3691,12 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
>
> if (data[IFLA_VXLAN_TTL_INHERIT]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - conf->flags |= VXLAN_F_TTL_INHERIT;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
> + VXLAN_F_TTL_INHERIT, changelink, false,
> + extack);
> + if (err)
> + return err;
> +
> }
>
> if (data[IFLA_VXLAN_LABEL])
> @@ -3658,10 +3704,11 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> IPV6_FLOWLABEL_MASK;
>
> if (data[IFLA_VXLAN_LEARNING]) {
> - if (nla_get_u8(data[IFLA_VXLAN_LEARNING]))
> - conf->flags |= VXLAN_F_LEARN;
> - else
> - conf->flags &= ~VXLAN_F_LEARN;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING,
> + VXLAN_F_LEARN, changelink, true,
> + extack);
> + if (err)
> + return err;
> } else if (!changelink) {
> /* default to learn on a new device */
> conf->flags |= VXLAN_F_LEARN;
> @@ -3671,44 +3718,52 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
>
> if (data[IFLA_VXLAN_PROXY]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_PROXY]))
> - conf->flags |= VXLAN_F_PROXY;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY,
> + VXLAN_F_PROXY, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_RSC]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_RSC]))
> - conf->flags |= VXLAN_F_RSC;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC,
> + VXLAN_F_RSC, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_L2MISS]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_L2MISS]))
> - conf->flags |= VXLAN_F_L2MISS;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS,
> + VXLAN_F_L2MISS, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_L3MISS]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_L3MISS]))
> - conf->flags |= VXLAN_F_L3MISS;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS,
> + VXLAN_F_L3MISS, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_LIMIT]) {
> - if (changelink)
> + if (changelink) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT],
> + "Cannot change limit");
> return -EOPNOTSUPP;
> + }
> conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]);
> }
>
> if (data[IFLA_VXLAN_COLLECT_METADATA]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA]))
> - conf->flags |= VXLAN_F_COLLECT_METADATA;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA,
> + VXLAN_F_COLLECT_METADATA, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_PORT_RANGE]) {
> @@ -3718,72 +3773,92 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> conf->port_min = ntohs(p->low);
> conf->port_max = ntohs(p->high);
> } else {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE],
> + "Cannot change port range");
> return -EOPNOTSUPP;
> }
> }
>
> if (data[IFLA_VXLAN_PORT]) {
> - if (changelink)
> + if (changelink) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT],
> + "Cannot change port");
> return -EOPNOTSUPP;
> + }
> conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]);
> }
>
> if (data[IFLA_VXLAN_UDP_CSUM]) {
> - if (changelink)
> + if (changelink) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_UDP_CSUM],
> + "Cannot change UDP_CSUM flag");
> return -EOPNOTSUPP;
> + }
> if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM]))
> conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX;
> }
>
> if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]))
> - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
> + VXLAN_F_UDP_ZERO_CSUM6_TX, changelink,
> + false, extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
> - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX,
> + VXLAN_F_UDP_ZERO_CSUM6_RX, changelink,
> + false, extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_REMCSUM_TX]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
> - conf->flags |= VXLAN_F_REMCSUM_TX;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX,
> + VXLAN_F_REMCSUM_TX, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_REMCSUM_RX]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
> - conf->flags |= VXLAN_F_REMCSUM_RX;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX,
> + VXLAN_F_REMCSUM_RX, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_GBP]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - conf->flags |= VXLAN_F_GBP;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP,
> + VXLAN_F_GBP, changelink, false, extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_GPE]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - conf->flags |= VXLAN_F_GPE;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE,
> + VXLAN_F_GPE, changelink, false,
> + extack);
> + if (err)
> + return err;
> }
>
> if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
> - if (changelink)
> - return -EOPNOTSUPP;
> - conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
> + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
> + VXLAN_F_REMCSUM_NOPARTIAL, changelink,
> + false, extack);
> + if (err)
> + return err;
> }
>
> if (tb[IFLA_MTU]) {
> - if (changelink)
> + if (changelink) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU],
> + "Cannot change mtu");
> return -EOPNOTSUPP;
> + }
> conf->mtu = nla_get_u32(tb[IFLA_MTU]);
> }
>
> @@ -3800,7 +3875,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
> struct vxlan_config conf;
> int err;
>
> - err = vxlan_nl2conf(tb, data, dev, &conf, false);
> + err = vxlan_nl2conf(tb, data, dev, &conf, false, extack);
> if (err)
> return err;
>
> @@ -3817,8 +3892,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
> struct vxlan_config conf;
> int err;
>
> - err = vxlan_nl2conf(tb, data,
> - dev, &conf, true);
> + err = vxlan_nl2conf(tb, data, dev, &conf, true, extack);
> if (err)
> return err;
>
> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 0976781..00254a5 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -453,4 +453,35 @@ vxlan_fdb_clear_offload(const struct net_device *dev, __be32 vni)
> }
> #endif
>
> +static inline void vxlan_flag_attr_error(int attrtype,
> + struct netlink_ext_ack *extack)
> +{
> +#define VXLAN_FLAG(flg) \
> + case IFLA_VXLAN_##flg: \
> + NL_SET_ERR_MSG_MOD(extack, \
> + "cannot change " #flg " flag"); \
> + break
> + switch (attrtype) {
> + VXLAN_FLAG(TTL_INHERIT);
> + VXLAN_FLAG(LEARNING);
> + VXLAN_FLAG(PROXY);
> + VXLAN_FLAG(RSC);
> + VXLAN_FLAG(L2MISS);
> + VXLAN_FLAG(L3MISS);
> + VXLAN_FLAG(COLLECT_METADATA);
> + VXLAN_FLAG(UDP_ZERO_CSUM6_TX);
> + VXLAN_FLAG(UDP_ZERO_CSUM6_RX);
> + VXLAN_FLAG(REMCSUM_TX);
> + VXLAN_FLAG(REMCSUM_RX);
> + VXLAN_FLAG(GBP);
> + VXLAN_FLAG(GPE);
> + VXLAN_FLAG(REMCSUM_NOPARTIAL);
> + default:
> + NL_SET_ERR_MSG_MOD(extack, \
> + "cannot change flag");
> + break;
> + }
> +#undef VXLAN_FLAG
> +}
> +
> #endif
^ permalink raw reply
* general protection fault in __xfrm_policy_check
From: syzbot @ 2019-02-26 13:54 UTC (permalink / raw)
To: davem, herbert, linux-kernel, netdev, steffen.klassert,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: ff7b11aa481f net: socket: set sock->sk to NULL after calli..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=12222e5cc00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7132344728e7ec3f
dashboard link: https://syzkaller.appspot.com/bug?extid=4ea28a8b817ee28bf324
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4ea28a8b817ee28bf324@syzkaller.appspotmail.com
kasan: CONFIG_KASAN_INLINE enabled
__sys_recvmmsg+0xe5/0x270 net/socket.c:2471
kasan: GPF could be caused by NULL-ptr deref or user memory access
__do_sys_recvmmsg net/socket.c:2492 [inline]
__se_sys_recvmmsg net/socket.c:2485 [inline]
__x64_sys_recvmmsg+0xe6/0x140 net/socket.c:2485
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
general protection fault: 0000 [#1] PREEMPT SMP KASAN
entry_SYSCALL_64_after_hwframe+0x49/0xbe
CPU: 1 PID: 16691 Comm: syz-executor.0 Not tainted 5.0.0-rc7+ #96
RIP: 0033:0x457e29
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RIP: 0010:xs_net include/net/xfrm.h:253 [inline]
RIP: 0010:xfrmi_decode_session net/xfrm/xfrm_interface.c:82 [inline]
RIP: 0010:xfrmi_decode_session+0x15c/0x6c0 net/xfrm/xfrm_interface.c:73
RSP: 002b:00007f715c86bc78 EFLAGS: 00000246 ORIG_RAX: 000000000000012b
Code: 7c fc 08 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 2e 05 00 00 48 b8 00
00 00 00 00 fc ff df 4f 8b 64 fc 08 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
85 01 05 00 00 4d 8b 3c 24 e8 71 34 5b fb e8 9c 70
RAX: ffffffffffffffda RBX: 00007f715c86bc90 RCX: 0000000000457e29
RSP: 0018:ffff88804594f120 EFLAGS: 00010246
RDX: 04000000000001de RSI: 00000000200037c0 RDI: 0000000000000004
RAX: dffffc0000000000 RBX: ffff88809fbd1d00 RCX: ffffc90005df3000
RBP: 000000000073bf00 R08: 0000000020003700 R09: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff86027000 RDI: ffff8880a0766b48
R10: 0000000000000006 R11: 0000000000000246 R12: 00007f715c86c6d4
RBP: ffff88804594f148 R08: ffff88805caa44c0 R09: ffffed1015d25bd0
R13: 00000000004c4b3e R14: 00000000004d8648 R15: 0000000000000005
R10: ffffed1015d25bcf R11: ffff8880ae92de7b R12: 0000000000000000
kobject: 'loop3' (0000000085c2aeb9): kobject_uevent_env
R13: 0000000000000037 R14: ffff88809fbd1d10 R15: ffffffffffffffff
kobject: 'loop3' (0000000085c2aeb9): fill_kobj_path: path
= '/devices/virtual/block/loop3'
FS: 00007f88e71e4700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004db9c0 CR3: 00000000908bb000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
kobject: 'loop3' (0000000085c2aeb9): kobject_uevent_env
__xfrm_policy_check+0x1f8/0x2730 net/xfrm/xfrm_policy.c:3316
kobject: 'loop3' (0000000085c2aeb9): fill_kobj_path: path
= '/devices/virtual/block/loop3'
kobject: 'loop2' (000000007df39881): kobject_uevent_env
__xfrm_policy_check2 include/net/xfrm.h:1176 [inline]
xfrm_policy_check include/net/xfrm.h:1181 [inline]
xfrm4_policy_check include/net/xfrm.h:1186 [inline]
vti_input+0x4e3/0x7b0 net/ipv4/ip_vti.c:63
vti_rcv+0x10b/0x140 net/ipv4/ip_vti.c:109
xfrm4_esp_rcv+0xd8/0x230 net/ipv4/xfrm4_protocol.c:100
ip_protocol_deliver_rcu+0x60/0x8e0 net/ipv4/ip_input.c:208
kobject: 'loop2' (000000007df39881): fill_kobj_path: path
= '/devices/virtual/block/loop2'
ip_local_deliver_finish+0x23b/0x390 net/ipv4/ip_input.c:234
NF_HOOK include/linux/netfilter.h:289 [inline]
NF_HOOK include/linux/netfilter.h:283 [inline]
ip_local_deliver+0x1e9/0x520 net/ipv4/ip_input.c:255
kobject: 'loop4' (00000000be00e036): kobject_uevent_env
dst_input include/net/dst.h:450 [inline]
ip_rcv_finish+0x1db/0x2f0 net/ipv4/ip_input.c:414
NF_HOOK include/linux/netfilter.h:289 [inline]
NF_HOOK include/linux/netfilter.h:283 [inline]
ip_rcv+0xe8/0x3f0 net/ipv4/ip_input.c:524
kobject: 'loop4' (00000000be00e036): fill_kobj_path: path
= '/devices/virtual/block/loop4'
__netif_receive_skb_one_core+0x115/0x1a0 net/core/dev.c:4973
__netif_receive_skb+0x2c/0x1c0 net/core/dev.c:5083
netif_receive_skb_internal+0x117/0x660 net/core/dev.c:5186
kobject: 'loop5' (0000000040ba3921): kobject_uevent_env
kobject: 'loop5' (0000000040ba3921): fill_kobj_path: path
= '/devices/virtual/block/loop5'
napi_frags_finish net/core/dev.c:5753 [inline]
napi_gro_frags+0xade/0xd10 net/core/dev.c:5827
tun_get_user+0x2c0e/0x3dd0 drivers/net/tun.c:1974
tun_chr_write_iter+0xbd/0x160 drivers/net/tun.c:2019
call_write_iter include/linux/fs.h:1863 [inline]
do_iter_readv_writev+0x5e0/0x8e0 fs/read_write.c:680
do_iter_write fs/read_write.c:956 [inline]
do_iter_write+0x184/0x610 fs/read_write.c:937
vfs_writev+0x1b3/0x2f0 fs/read_write.c:1001
do_writev+0xf6/0x290 fs/read_write.c:1036
__do_sys_writev fs/read_write.c:1109 [inline]
__se_sys_writev fs/read_write.c:1106 [inline]
__x64_sys_writev+0x75/0xb0 fs/read_write.c:1106
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457ce1
Code: 75 14 b8 14 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 e4 b9 fb ff c3 48
83 ec 08 e8 1a 2d 00 00 48 89 04 24 b8 14 00 00 00 0f 05 <48> 8b 3c 24 48
89 c2 e8 63 2d 00 00 48 89 d0 48 83 c4 08 48 3d 01
kobject: 'loop1' (000000005642785e): kobject_uevent_env
RSP: 002b:00007f88e71e3ba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 000000000000003e RCX: 0000000000457ce1
RDX: 0000000000000001 RSI: 00007f88e71e3bf0 RDI: 00000000000000f0
RBP: 0000000020000040 R08: 00000000000000f0 R09: 0000000000000000
R10: 00007f88e71e49d0 R11: 0000000000000293 R12: 00007f88e71e46d4
R13: 00000000004c64e1 R14: 00000000004db9c0 R15: 00000000ffffffff
Modules linked in:
---[ end trace 2fbdf2db5ff7df79 ]---
kobject: 'loop1' (000000005642785e): fill_kobj_path: path
= '/devices/virtual/block/loop1'
RIP: 0010:xs_net include/net/xfrm.h:253 [inline]
RIP: 0010:xfrmi_decode_session net/xfrm/xfrm_interface.c:82 [inline]
RIP: 0010:xfrmi_decode_session+0x15c/0x6c0 net/xfrm/xfrm_interface.c:73
Code: 7c fc 08 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 2e 05 00 00 48 b8 00
00 00 00 00 fc ff df 4f 8b 64 fc 08 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
85 01 05 00 00 4d 8b 3c 24 e8 71 34 5b fb e8 9c 70
kobject: 'loop3' (0000000085c2aeb9): kobject_uevent_env
RSP: 0018:ffff88804594f120 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffff88809fbd1d00 RCX: ffffc90005df3000
RDX: 0000000000000000 RSI: ffffffff86027000 RDI: ffff8880a0766b48
kobject: 'loop3' (0000000085c2aeb9): fill_kobj_path: path
= '/devices/virtual/block/loop3'
RBP: ffff88804594f148 R08: ffff88805caa44c0 R09: ffffed1015d25bd0
R10: ffffed1015d25bcf R11: ffff8880ae92de7b R12: 0000000000000000
R13: 0000000000000037 R14: ffff88809fbd1d10 R15: ffffffffffffffff
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
FS: 00007f88e71e4700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CPU: 0 PID: 16722 Comm: syz-executor.2 Tainted: G D
5.0.0-rc7+ #96
CR2: 00000000004db9c0 CR3: 00000000908bb000 CR4: 00000000001406e0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
fail_dump lib/fault-inject.c:51 [inline]
should_fail.cold+0xa/0x1b lib/fault-inject.c:149
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
__should_failslab+0x121/0x190 mm/failslab.c:32
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
should_failslab+0x9/0x14 mm/slab_common.c:1604
slab_pre_alloc_hook mm/slab.h:423 [inline]
slab_alloc_node mm/slab.c:3295 [inline]
kmem_cache_alloc_node+0x56/0x710 mm/slab.c:3637
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero
From: Roi Dayan @ 2019-02-26 13:54 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-2-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> When max modify header action is zero, we return -EOPNOTSUPP
> directly. In this way, we can ignore wrong message info (e.g.
> "mlx5: parsed 0 pedit actions, can't do more").
>
> This happens when offloading pedit actions on mlx VFs.
>
> For example:
> $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
> flower skip_sw dst_mac 00:10:56:fb:64:e8 \
> dst_ip 1.1.1.100 src_ip 1.1.1.200 \
> action pedit ex munge eth src set 00:10:56:b4:5d:20
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index b38986e..708f819 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2002,7 +2002,8 @@ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
> static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> struct pedit_headers_action *hdrs,
> int namespace,
> - struct mlx5e_tc_flow_parse_attr *parse_attr)
> + struct mlx5e_tc_flow_parse_attr *parse_attr,
> + struct netlink_ext_ack *extack)
> {
> int nkeys, action_size, max_actions;
>
> @@ -2015,6 +2016,12 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
>
> + if (!max_actions) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "don't support pedit actions, can't offload");
can we rephrase that to match the msg style you did in patch 5 ?
i.e. The pedit offload action is not supported
> + return -EOPNOTSUPP;
> + }
> +
> /* can get up to crazingly 16 HW actions in 32 bits pedit SW key */
> max_actions = min(max_actions, nkeys * 16);
>
> @@ -2072,7 +2079,8 @@ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
> u8 cmd;
>
> if (!parse_attr->mod_hdr_actions) {
> - err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
> + err = alloc_mod_hdr_actions(priv, hdrs,
> + namespace, parse_attr, extack);
> if (err)
> goto out_err;
> }
>
^ permalink raw reply
* Re: [PATCH] net: phy: Micrel KSZ8061: link failure after cable connect
From: Andrew Lunn @ 2019-02-26 14:06 UTC (permalink / raw)
To: Rajasingh Thavamani
Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20190226081506.25887-1-T.Rajasingh@landisgyr.com>
On Tue, Feb 26, 2019 at 01:45:06PM +0530, Rajasingh Thavamani wrote:
> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf descripes the problem and possible workarounds in
> detail, see below.
> The batch implements workaround 1, which permanently fixes the issue.
...
> PLAN
> This errata will not be corrected in the future revision.
>
> Signed-off-by: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
We have been here before:
https://lkml.org/lkml/2018/11/23/466
At that time, the patch was from Alexander Onnasch, and has his
signed-off-by. Now it only has your SOB. It probably should have
both. My reviewed-by has gone missing. And this is version 3? And in
the comments i also included a Fixed: tag, which should be added.
Please submit a v4 with all these issues fixed.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink
From: Roopa Prabhu @ 2019-02-26 14:06 UTC (permalink / raw)
To: Petr Machata
Cc: davem@davemloft.net, netdev@vger.kernel.org,
dsa@cumulusnetworks.com, sd@queasysnail.net,
johannes@sipsolutions.net
In-Reply-To: <87imx6fyk1.fsf@mellanox.com>
On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote:
>
>
> Roopa Prabhu <roopa@cumulusnetworks.com> writes:
>
> > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >
> > This patch adds extack coverage in vxlan link
> > create and changelink paths. Introduces a new helper
> > vxlan_nl2flags to consolidate flag attribute validation.
> >
> > thanks to Johannes Berg for some tips to construct the
> > generic vxlan flag extack strings.
> >
> > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> > ---
> > drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++-----------------
> > include/net/vxlan.h | 31 ++++++++
> > 2 files changed, 172 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index 577201c..a3c46d7 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
> > return err;
> > }
> >
> > +/* Set/clear flags based on attribute */
> > +static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
> > + int attrtype, unsigned long mask, bool changelink,
> > + bool changelink_supported,
> > + struct netlink_ext_ack *extack)
> > +{
> > + unsigned long flags;
> > +
> > + if (!tb[attrtype])
> > + return 0;
> > +
> > + if (changelink && !changelink_supported) {
> > + vxlan_flag_attr_error(attrtype, extack);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (vxlan_policy[attrtype].type == NLA_FLAG)
> > + flags = conf->flags | mask;
> > + else if (nla_get_u8(tb[attrtype]))
> > + flags = conf->flags | mask;
> > + else
> > + flags = conf->flags & ~mask;
>
> Many of the flags for which you call this don't actually have the else
> branch. However I suspect there are no good reasons not to allow
> resetting a flag.
>
> Reviewed-by: Petr Machata <petrm@mellanox.com>
yes, correct, that was intentional.
also, I am not sure what is the best way to support reseting of a NLA_FLAG.
Absence of the flag attribute in the request cannot be the reason for
resetting or clearing the flag.
None of the NLA_FLAG attributes support changing the flag today. This
patch does not change that.
^ permalink raw reply
* Re: [PATCH net 2/4] tls: Fix write space handling
From: Boris Pismenny @ 2019-02-26 14:13 UTC (permalink / raw)
To: Vakul Garg, Aviad Yehezkel, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: Eran Ben Elisha
In-Reply-To: <DB7PR04MB425215CB92707A2FD5CD6A138B7B0@DB7PR04MB4252.eurprd04.prod.outlook.com>
On 2/26/2019 2:49 PM, Vakul Garg wrote:
>
>
>> -----Original Message-----
>> From: Boris Pismenny <borisp@mellanox.com>
>> Sent: Tuesday, February 26, 2019 5:43 PM
>> To: aviadye@mellanox.com; davejwatson@fb.com;
>> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
>> <vakul.garg@nxp.com>; netdev@vger.kernel.org
>> Cc: eranbe@mellanox.com; borisp@mellanox.com
>> Subject: [PATCH net 2/4] tls: Fix write space handling
>>
>> TLS device cannot use the sw context. This patch returns the original
>> tls device write space handler and moves the sw/device specific portions
>> to the relevant files.
>>
>> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records
>> for performance")
>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
>> ---
>> include/net/tls.h | 3 +++
>> net/tls/tls_device.c | 16 ++++++++++++++++
>> net/tls/tls_main.c | 17 +++++++++--------
>> net/tls/tls_sw.c | 15 +++++++++++++++
>> 4 files changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/net/tls.h b/include/net/tls.h
>> index a528a082da73..9d7c53737b13 100644
>> --- a/include/net/tls.h
>> +++ b/include/net/tls.h
>> @@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock
>> *sk)
>> return !!tls_sw_ctx_tx(ctx);
>> }
>>
>> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
>> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
>> +
>> static inline struct tls_offload_context_rx *
>> tls_offload_ctx_rx(const struct tls_context *tls_ctx)
>> {
>> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
>> index 3e5e8e021a87..e8988b3f3236 100644
>> --- a/net/tls/tls_device.c
>> +++ b/net/tls/tls_device.c
>> @@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct
>> sock *sk, int flags)
>> return tls_push_data(sk, &msg_iter, 0, flags,
>> TLS_RECORD_TYPE_DATA);
>> }
>>
>> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
>> +{
>> + int rc = 0;
>> +
>> + if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
>> + gfp_t sk_allocation = sk->sk_allocation;
>> +
>> + sk->sk_allocation = GFP_ATOMIC;
>> + rc = tls_push_partial_record(sk, ctx,
>> + MSG_DONTWAIT |
>> MSG_NOSIGNAL);
>> + sk->sk_allocation = sk_allocation;
>> + }
>> +
>> + return rc;
>> +}
>> +
>> void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
>> {
>> struct tls_context *tls_ctx = tls_get_ctx(sk);
>> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
>> index 7e05af75536d..11c1980a75cb 100644
>> --- a/net/tls/tls_main.c
>> +++ b/net/tls/tls_main.c
>> @@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct
>> tls_context *ctx,
>> static void tls_write_space(struct sock *sk)
>> {
>> struct tls_context *ctx = tls_get_ctx(sk);
>> - struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
>> + int rc;
>>
>> /* If in_tcp_sendpages call lower protocol write space handler
>> * to ensure we wake up any waiting operations there. For example
>> @@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
>> return;
>> }
>>
>> - /* Schedule the transmission if tx list is ready */
>> - if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
>> - /* Schedule the transmission */
>> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx-
>>> tx_bitmask))
>> - schedule_delayed_work(&tx_ctx->tx_work.work, 0);
>> - }
>> +#ifdef CONFIG_TLS_DEVICE
>> + if (ctx->tx_conf == TLS_HW)
>> + rc = tls_device_write_space(sk, ctx);
>> + else
>> +#endif
>> + rc = tls_sw_write_space(sk, ctx);
>>
>> - ctx->sk_write_space(sk);
>> + if (!rc)
>
> Why do we need to check 'rc'?
>
> If it is required, then ' ctx->sk_write_space(sk)' can move to tls_device_write_space()
> since tls_sw_write_space() always returns '0'.
>
It is not necessary in the software code path due to the delayed work
that is there. But, we need in the device flow. I'll move it there.
>> + ctx->sk_write_space(sk);
>> }
>>
>> static void tls_ctx_free(struct tls_context *ctx)
>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
>> index 1cc830582fa8..4afa67b00aaf 100644
>> --- a/net/tls/tls_sw.c
>> +++ b/net/tls/tls_sw.c
>> @@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct
>> *work)
>> release_sock(sk);
>> }
>>
>> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
>> +{
>> + struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
>> +
>> + /* Schedule the transmission if tx list is ready */
>> + if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
>> + /* Schedule the transmission */
>> + if (!test_and_set_bit(BIT_TX_SCHEDULED,
>> + &tx_ctx->tx_bitmask))
>> + schedule_delayed_work(&tx_ctx->tx_work.work, 0);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
>> {
>> struct tls_context *tls_ctx = tls_get_ctx(sk);
>> --
>> 2.12.2
>
^ permalink raw reply
* [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: zerons @ 2019-02-26 14:15 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linux-kernel
[ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
In bpf/syscall.c, bpf_map_get_fd_by_id() use bpf_map_inc_not_zero() to increase
the refcount, both map->refcnt and map->usercnt. Then, if bpf_map_new_fd() fails,
should handle map->usercnt too.
Signed-off-by: zerons <sironhide0null@gmail.com>
---
kernel/bpf/syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cf5040f..db1ed12 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1966,7 +1966,7 @@ static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
fd = bpf_map_new_fd(map, f_flags);
if (fd < 0)
- bpf_map_put(map);
+ bpf_map_put_with_uref(map);
return fd;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net] selftests: fixes for UDP GRO
From: Paolo Abeni @ 2019-02-26 14:27 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn
The current implementation for UDP GRO tests is racy: the receiver
may flush the RX queue while the sending is still transmitting and
incorrectly report RX errors, with a wrong number of packet received.
Add explicit timeouts to the receiver for both connection activation
(first packet received for UDP) and reception completion, so that
in the above critical scenario the receiver will wait for the
transfer completion.
Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
tools/testing/selftests/net/udpgro.sh | 8 ++--
tools/testing/selftests/net/udpgso_bench_rx.c | 42 +++++++++++++------
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh
index aeac53a99aeb..ac2a30be9b32 100755
--- a/tools/testing/selftests/net/udpgro.sh
+++ b/tools/testing/selftests/net/udpgro.sh
@@ -37,7 +37,7 @@ run_one() {
cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx ${rx_args} && \
+ ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} && \
echo "ok" || \
echo "failed" &
@@ -81,7 +81,7 @@ run_one_nat() {
# will land on the 'plain' one
ip netns exec "${PEER_NS}" ./udpgso_bench_rx -G ${family} -b ${addr1} -n 0 &
pid=$!
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx ${family} -b ${addr2%/*} ${rx_args} && \
+ ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${family} -b ${addr2%/*} ${rx_args} && \
echo "ok" || \
echo "failed"&
@@ -99,8 +99,8 @@ run_one_2sock() {
cfg_veth
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx ${rx_args} -p 12345 &
- ip netns exec "${PEER_NS}" ./udpgso_bench_rx ${rx_args} && \
+ ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 1000 -R 10 ${rx_args} -p 12345 &
+ ip netns exec "${PEER_NS}" ./udpgso_bench_rx -C 2000 -R 10 ${rx_args} && \
echo "ok" || \
echo "failed" &
diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c
index 0c960f673324..db3d4a8b5a4c 100644
--- a/tools/testing/selftests/net/udpgso_bench_rx.c
+++ b/tools/testing/selftests/net/udpgso_bench_rx.c
@@ -45,6 +45,8 @@ static int cfg_alen = sizeof(struct sockaddr_in6);
static int cfg_expected_pkt_nr;
static int cfg_expected_pkt_len;
static int cfg_expected_gso_size;
+static int cfg_connect_timeout_ms;
+static int cfg_rcv_timeout_ms;
static struct sockaddr_storage cfg_bind_addr;
static bool interrupted;
@@ -87,7 +89,7 @@ static unsigned long gettimeofday_ms(void)
return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
}
-static void do_poll(int fd)
+static void do_poll(int fd, int timeout_ms)
{
struct pollfd pfd;
int ret;
@@ -102,8 +104,16 @@ static void do_poll(int fd)
break;
if (ret == -1)
error(1, errno, "poll");
- if (ret == 0)
- continue;
+ if (ret == 0) {
+ if (!timeout_ms)
+ continue;
+
+ timeout_ms -= 10;
+ if (timeout_ms <= 0) {
+ interrupted = true;
+ break;
+ }
+ }
if (pfd.revents != POLLIN)
error(1, errno, "poll: 0x%x expected 0x%x\n",
pfd.revents, POLLIN);
@@ -134,7 +144,7 @@ static int do_socket(bool do_tcp)
if (listen(accept_fd, 1))
error(1, errno, "listen");
- do_poll(accept_fd);
+ do_poll(accept_fd, cfg_connect_timeout_ms);
if (interrupted)
exit(0);
@@ -273,7 +283,9 @@ static void do_flush_udp(int fd)
static void usage(const char *filepath)
{
- error(1, 0, "Usage: %s [-Grtv] [-b addr] [-p port] [-l pktlen] [-n packetnr] [-S gsosize]", filepath);
+ error(1, 0, "Usage: %s [-C connect_timeout] [-Grtv] [-b addr] [-p port]"
+ " [-l pktlen] [-n packetnr] [-R rcv_timeout] [-S gsosize]",
+ filepath);
}
static void parse_opts(int argc, char **argv)
@@ -282,7 +294,7 @@ static void parse_opts(int argc, char **argv)
/* bind to any by default */
setup_sockaddr(PF_INET6, "::", &cfg_bind_addr);
- while ((c = getopt(argc, argv, "4b:Gl:n:p:rS:tv")) != -1) {
+ while ((c = getopt(argc, argv, "4b:C:Gl:n:p:rR:S:tv")) != -1) {
switch (c) {
case '4':
cfg_family = PF_INET;
@@ -292,6 +304,9 @@ static void parse_opts(int argc, char **argv)
case 'b':
setup_sockaddr(cfg_family, optarg, &cfg_bind_addr);
break;
+ case 'C':
+ cfg_connect_timeout_ms = strtoul(optarg, NULL, 0);
+ break;
case 'G':
cfg_gro_segment = true;
break;
@@ -307,6 +322,9 @@ static void parse_opts(int argc, char **argv)
case 'r':
cfg_read_all = true;
break;
+ case 'R':
+ cfg_rcv_timeout_ms = strtoul(optarg, NULL, 0);
+ break;
case 'S':
cfg_expected_gso_size = strtol(optarg, NULL, 0);
break;
@@ -329,8 +347,9 @@ static void parse_opts(int argc, char **argv)
static void do_recv(void)
{
+ int timeout_ms = cfg_tcp ? cfg_rcv_timeout_ms : cfg_connect_timeout_ms;
unsigned long tnow, treport;
- int fd, loop = 0;
+ int fd;
fd = do_socket(cfg_tcp);
@@ -342,12 +361,7 @@ static void do_recv(void)
treport = gettimeofday_ms() + 1000;
do {
- /* force termination after the second poll(); this cope both
- * with sender slower than receiver and missing packet errors
- */
- if (cfg_expected_pkt_nr && loop++)
- interrupted = true;
- do_poll(fd);
+ do_poll(fd, timeout_ms);
if (cfg_tcp)
do_flush_tcp(fd);
@@ -365,6 +379,8 @@ static void do_recv(void)
treport = tnow + 1000;
}
+ timeout_ms = cfg_rcv_timeout_ms;
+
} while (!interrupted);
if (cfg_expected_pkt_nr && (packets != cfg_expected_pkt_nr))
--
2.20.1
^ 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