* [PATCH net-next 1/6] nfp: flower: set ip tunnel ttl from encap action
From: Simon Horman @ 2018-08-07 15:35 UTC (permalink / raw)
To: David Miller; +Cc: Jiri Pirko, Cong Wang, Jakub Kicinski, netdev, oss-drivers
In-Reply-To: <20180807153603.1815-1-simon.horman@netronome.com>
From: John Hurley <john.hurley@netronome.com>
The TTL for encapsulating headers in IPv4 UDP tunnels is taken from a
route lookup. Modify this to first check if a user has specified a TTL to
be used in the TC action.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/flower/action.c | 39 ++++++++++++----------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index e56b815a8dc6..a79d078ab3e8 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -238,18 +238,12 @@ nfp_fl_set_ipv4_udp_tun(struct nfp_fl_set_ipv4_udp_tun *set_tun,
size_t act_size = sizeof(struct nfp_fl_set_ipv4_udp_tun);
struct ip_tunnel_info *ip_tun = tcf_tunnel_info(action);
u32 tmp_set_ip_tun_type_index = 0;
- struct flowi4 flow = {};
/* Currently support one pre-tunnel so index is always 0. */
int pretun_idx = 0;
- struct rtable *rt;
- struct net *net;
- int err;
if (ip_tun->options_len)
return -EOPNOTSUPP;
- net = dev_net(netdev);
-
set_tun->head.jump_id = NFP_FL_ACTION_OPCODE_SET_IPV4_TUNNEL;
set_tun->head.len_lw = act_size >> NFP_FL_LW_SIZ;
@@ -261,19 +255,28 @@ nfp_fl_set_ipv4_udp_tun(struct nfp_fl_set_ipv4_udp_tun *set_tun,
set_tun->tun_type_index = cpu_to_be32(tmp_set_ip_tun_type_index);
set_tun->tun_id = ip_tun->key.tun_id;
- /* Do a route lookup to determine ttl - if fails then use default.
- * Note that CONFIG_INET is a requirement of CONFIG_NET_SWITCHDEV so
- * must be defined here.
- */
- flow.daddr = ip_tun->key.u.ipv4.dst;
- flow.flowi4_proto = IPPROTO_UDP;
- rt = ip_route_output_key(net, &flow);
- err = PTR_ERR_OR_ZERO(rt);
- if (!err) {
- set_tun->ttl = ip4_dst_hoplimit(&rt->dst);
- ip_rt_put(rt);
+ if (ip_tun->key.ttl) {
+ set_tun->ttl = ip_tun->key.ttl;
} else {
- set_tun->ttl = net->ipv4.sysctl_ip_default_ttl;
+ struct net *net = dev_net(netdev);
+ struct flowi4 flow = {};
+ struct rtable *rt;
+ int err;
+
+ /* Do a route lookup to determine ttl - if fails then use
+ * default. Note that CONFIG_INET is a requirement of
+ * CONFIG_NET_SWITCHDEV so must be defined here.
+ */
+ flow.daddr = ip_tun->key.u.ipv4.dst;
+ flow.flowi4_proto = IPPROTO_UDP;
+ rt = ip_route_output_key(net, &flow);
+ err = PTR_ERR_OR_ZERO(rt);
+ if (!err) {
+ set_tun->ttl = ip4_dst_hoplimit(&rt->dst);
+ ip_rt_put(rt);
+ } else {
+ set_tun->ttl = net->ipv4.sysctl_ip_default_ttl;
+ }
}
set_tun->tos = ip_tun->key.tos;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 0/6] nfp: flower: tunnel TTL & TOS, and Geneve options set & match support
From: Simon Horman @ 2018-08-07 15:35 UTC (permalink / raw)
To: David Miller; +Cc: Jiri Pirko, Cong Wang, Jakub Kicinski, netdev, oss-drivers
Hi Dave,
this series contains updates for the TC Flower classifier
and the offload facility for it in the NFP driver.
* Patches 1 & 2: update the NFP driver to allow offload
of matching and setting tunnel ToS/TTL of flows using the TC Flower
classifier and tun_key action
* Patches 3 & 4: enhance the flow dissector and TC Flower classifier
to allow match on Geneve options
* Patch 5 & 6: update the NFP driver to allow offload of
matching and setting Geneve options of flows using the TC Flower
classifier and tun_key action
John Hurley (2):
nfp: flower: set ip tunnel ttl from encap action
nfp: flower: allow matching on ipv4 UDP tunnel tos and ttl
Pieter Jansen van Vuuren (3):
net/sched: allow flower to match tunnel options
nfp: flower: add geneve option push action offload
nfp: flower: add geneve option match offload
Simon Horman (1):
flow_dissector: allow dissection of tunnel options from metadata
drivers/net/ethernet/netronome/nfp/flower/action.c | 139 ++++++++++--
drivers/net/ethernet/netronome/nfp/flower/cmsg.h | 33 ++-
drivers/net/ethernet/netronome/nfp/flower/main.h | 1 +
drivers/net/ethernet/netronome/nfp/flower/match.c | 34 +++
.../net/ethernet/netronome/nfp/flower/offload.c | 42 +++-
include/net/flow_dissector.h | 17 ++
include/uapi/linux/pkt_cls.h | 26 +++
net/core/flow_dissector.c | 19 +-
net/sched/cls_flower.c | 244 ++++++++++++++++++++-
9 files changed, 525 insertions(+), 30 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH v3 net-next 5/9] net: stmmac: Add MDIO related functions for XGMAC2
From: Jose Abreu @ 2018-08-07 15:29 UTC (permalink / raw)
To: Florian Fainelli, Jose Abreu, netdev
Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
Andrew Lunn
In-Reply-To: <3a097ddf-cff1-460f-2c63-7347274c1ee5@synopsys.com>
On 07-08-2018 14:35, Jose Abreu wrote:
> Hi Florian,
>
> On 06-08-2018 16:25, Florian Fainelli wrote:
>> On August 6, 2018 12:59:54 AM PDT, Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>>> On 03-08-2018 20:06, Florian Fainelli wrote:
>>>> On 08/03/2018 08:50 AM, Jose Abreu wrote:
>>>>> Add the MDIO related funcionalities for the new IP block XGMAC2.
>>>>>
>>>>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>>>>> Cc: David S. Miller <davem@davemloft.net>
>>>>> Cc: Joao Pinto <jpinto@synopsys.com>
>>>>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>>>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>>>> ---
>>>>> +satic int stmmac_xgmac2_c22_format(struct stmmac_priv *priv, int
>>> phyaddr,
>>>>> + int phyreg, u32 *hw_addr)
>>>>> +{
>>>>> + unsigned int mii_data = priv->hw->mii.data;
>>>>> + u32 tmp;
>>>>> +
>>>>> + /* HW does not support C22 addr >= 4 */
>>>>> + if (phyaddr >= 4)
>>>>> + return -ENODEV;
>>>> It would be nice if this could be moved at probe time so you don't
>>> have
>>>> to wait until you connect to the PHY, read its PHY OUI and find out
>>> it
>>>> has a MDIO address >= 4. Not a blocker, but something that could be
>>>> improved further on.
>>>>
>>>> In premise you could even scan the MDIO bus' device tree node, and
>>> find
>>>> that out ahead of time.
>>> Oh, but I use phylib ... I only provide the read/write callbacks
>>> so I think we should avoid duplicating the code that's already in
>>> the phylib ... No?
>> You can always extract the code that scans a MDIO bus into a helper function and make it parametrized with a callback of some kind. In that case I would be fine with you open coding the MDIO bus scan to find out if there is an address >= 4.
> Sorry but I dont' think thats the best solution because
> of_mdiobus_register() already scans the bus.
My mistake. Its stmmac thats scanning the bus. See this:
359 for (addr = 0; addr < PHY_MAX_ADDR; addr++)
{
360 struct phy_device *phydev =
mdiobus_get_phy(new_bus, addr);
361
362 if
(!phydev)
363
continue;
So, its just cycling through all possible phys (0..31) ... Do you
think it would be okay if I just limit the cycle max to 4 for
this xgmac2 for now ? I would maintain the if in the
stmmac_xgmac2_c22_format though, as safe-guard.
Thanks and Best Regards,
Jose Miguel Abreu
> Duplicating this
> should be avoided, no?
>
> Note all of this is probably never needed because stmmac just
> picks the first phy it finds, if phy_addr is not specified ...
>
>>>>> + /* Wait until any existing MII operation is complete */
>>>>> + if (readl_poll_timeout(priv->ioaddr + mii_data, tmp,
>>>>> + !(tmp & MII_XGMAC_BUSY), 100, 10000))
>>>>> + return -EBUSY;
>>>>> +
>>>>> + /* Set port as Clause 22 */
>>>>> + tmp = readl(priv->ioaddr + XGMAC_MDIO_C22P);
>>>>> + tmp |= BIT(phyaddr);
>>>> Since the registers are being Read/Modify/Write here, don't you need
>>> to
>>>> clear the previous address bits as well?
>>>>
>>>> You probably did not encounter any problems in your testing if you
>>> had
>>>> only one PHY on the MDIO bus, but this is not something that is
>>>> necessarily true, e.g: if you have an Ethernet switch, several MDIO
>>> bus
>>>> addresses are going to be responding.
>>>>
>>>> Your MDIO bus implementation must be able to support one transaction
>>>> with one PHY address and the next transaction with another PHY
>>> address ,
>>>> etc...
>>>>
>>>> That is something that should be easy to fix and be resubmitted as
>>> part
>>>> of v4.
>>> I'm not following you here... I only set/unset the bit for the
>>> corresponding phyaddr that phylib wants to read/write. Why would
>>> I clear the remaining addresses?
>> Because this is all about transactions, the HW must be in a state that it will be able to perform that transaction correctly. So here for instance if you needed to support a C45 transaction you would have to clear that bit for that particular PHY address. Since you don't appear to support those yet then yes the code appears fine though it would not hurt if you did clear all other PHY's c22 bits to make it clear what this does.
> I can't test C45 right now but I will in a near future. In that
> case then we will need to support C22 and C45 so I may want to
> only set one bit for a specific phy that only supports c22.
>
> Thanks and Best Regards,
> Jose Miguel Abreu
>
^ permalink raw reply
* Re: [PATCH net-next] ieee802154: hwsim: fix rcu address annotation
From: Stefan Schmidt @ 2018-08-07 15:28 UTC (permalink / raw)
To: Alexander Aring, netdev; +Cc: linux-wpan, kernel
In-Reply-To: <20180807143444.32224-1-aring@mojatatu.com>
Hello Dave.
On 08/07/2018 04:34 PM, Alexander Aring wrote:
> This patch fixes the following sparse warning about mismatch rcu
> attribute for address space annotation:
>
> ...
> error: incompatible types in comparison expression (different modifiers)
> error: incompatible types in comparison expression (different address spaces)
> ...
>
> Some __rcu annotation was at non-pointers list head structures and one was
> missing in edge information which is used by rcu_assign_pointer() to
> update edge setting information.
>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
> Hopefully I did it right now. Sorry again.
>
> drivers/net/ieee802154/mac802154_hwsim.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index 1982308b9b1c..f4e92054f7df 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -36,7 +36,7 @@ MODULE_LICENSE("GPL");
> static LIST_HEAD(hwsim_phys);
> static DEFINE_MUTEX(hwsim_phys_lock);
>
> -static __rcu LIST_HEAD(hwsim_ifup_phys);
> +static LIST_HEAD(hwsim_ifup_phys);
>
> static struct platform_device *mac802154hwsim_dev;
>
> @@ -68,7 +68,7 @@ struct hwsim_edge_info {
>
> struct hwsim_edge {
> struct hwsim_phy *endpoint;
> - struct hwsim_edge_info *info;
> + struct hwsim_edge_info __rcu *info;
>
> struct list_head list;
> struct rcu_head rcu;
> @@ -81,7 +81,7 @@ struct hwsim_phy {
> struct hwsim_pib __rcu *pib;
>
> bool suspended;
> - struct list_head __rcu edges;
> + struct list_head edges;
>
> struct list_head list;
> struct list_head list_ifup;
>
Could you apply this patch directly to net-next?
It fixes an issue introduced with my pull request from two days ago. I
have nothing else to submit so I would appreciate if you could apply it
directly.
Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>
regards
Stefan Schmidt
^ permalink raw reply
* RE: [PATCH RFC net-next 1/1] net/tls: Combined memory allocation for decryption request
From: Vakul Garg @ 2018-08-07 15:24 UTC (permalink / raw)
To: Dave Watson
Cc: netdev@vger.kernel.org, borisp@mellanox.com, aviadye@mellanox.com,
davem@davemloft.net
In-Reply-To: <20180807145618.GA91350@mengjuew-mbp.dhcp.thefacebook.com>
> -----Original Message-----
> From: Dave Watson [mailto:davejwatson@fb.com]
> Sent: Tuesday, August 7, 2018 8:26 PM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davem@davemloft.net
> Subject: Re: [PATCH RFC net-next 1/1] net/tls: Combined memory allocation
> for decryption request
>
> Hi Vakul,
>
> Only minor comments, mostly looks good to me. Thanks
>
> > +/* This function decrypts the input skb into either out_iov or in
> > +out_sg
> > + * or in skb buffers itself. The input parameter 'zc' indicates if
> > + * zero-copy mode needs to be tried or not. With zero-copy mode,
> > +either
> > + * out_iov or out_sg must be non-NULL. In case both out_iov and
> > +out_sg are
> > + * NULL, then the decryption happens inside skb buffers itself, i.e.
> > + * zero-copy gets disabled and 'zc' is updated.
> > + */
> > +
> > +static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
> > + struct iov_iter *out_iov,
> > + struct scatterlist *out_sg,
> > + int *chunk, bool *zc)
> > +{
> > + struct tls_context *tls_ctx = tls_get_ctx(sk);
> > + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> > + struct strp_msg *rxm = strp_msg(skb);
> > + int n_sgin, n_sgout, nsg, mem_size, aead_size, err, pages = 0;
> > + struct aead_request *aead_req;
> > + struct sk_buff *unused;
> > + u8 *aad, *iv, *mem = NULL;
> > + struct scatterlist *sgin = NULL;
> > + struct scatterlist *sgout = NULL;
> > + const int data_len = rxm->full_len - tls_ctx->rx.overhead_size;
> > +
> > + if (*zc && (out_iov || out_sg)) {
> > + if (out_iov)
> > + n_sgout = iov_iter_npages(out_iov, INT_MAX) + 1;
> > + else if (out_sg)
> > + n_sgout = sg_nents(out_sg);
> > + else
> > + goto no_zerocopy;
>
> Is the last else necessary? It looks like the if already checks for out_iov ||
> out_sg.
>
You are right. I can remove 'else. 'else if' can also be replaced with 'else'.
I would submit v2.
> > struct scatterlist *sgout)
> > {
> > - struct tls_context *tls_ctx = tls_get_ctx(sk);
> > - struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> > - char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + MAX_IV_SIZE];
> > - struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
> > - struct scatterlist *sgin = &sgin_arr[0];
> > - struct strp_msg *rxm = strp_msg(skb);
> > - int ret, nsg;
> > - struct sk_buff *unused;
> > -
> > - ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
> > - iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> > - tls_ctx->rx.iv_size);
> > - if (ret < 0)
> > - return ret;
> > -
> > - memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > - if (!sgout) {
> > - nsg = skb_cow_data(skb, 0, &unused);
> > - } else {
> > - nsg = skb_nsg(skb,
> > - rxm->offset + tls_ctx->rx.prepend_size,
> > - rxm->full_len - tls_ctx->rx.prepend_size);
> > - if (nsg <= 0)
> > - return nsg;
> > - }
> > -
> > - // We need one extra for ctx->rx_aad_ciphertext
> > - nsg++;
> > -
> > - if (nsg > ARRAY_SIZE(sgin_arr))
> > - sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
> > -
> > - if (!sgout)
> > - sgout = sgin;
> > -
> > - sg_init_table(sgin, nsg);
> > - sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
> > -
> > - nsg = skb_to_sgvec(skb, &sgin[1],
> > - rxm->offset + tls_ctx->rx.prepend_size,
> > - rxm->full_len - tls_ctx->rx.prepend_size);
> > - if (nsg < 0) {
> > - ret = nsg;
> > - goto out;
> > - }
> > -
> > - tls_make_aad(ctx->rx_aad_ciphertext,
> > - rxm->full_len - tls_ctx->rx.overhead_size,
> > - tls_ctx->rx.rec_seq,
> > - tls_ctx->rx.rec_seq_size,
> > - ctx->control);
> > -
> > - ret = tls_do_decryption(sk, sgin, sgout, iv,
> > - rxm->full_len - tls_ctx->rx.overhead_size,
> > - skb, sk->sk_allocation);
> > -
> > -out:
> > - if (sgin != &sgin_arr[0])
> > - kfree(sgin);
> > + bool zc = true;
> > + int chunk;
> >
> > - return ret;
> > + return decrypt_internal(sk, skb, NULL, sgout, &chunk, &zc);
> > }
>
> Can we merge this function to callsites? It's pretty useless now.
I retained decrypt_skb() in its existing prototype to keep the caller
in tls_device.c unmodified. It seemed simpler that way to me
from code readability perspective in tls_device.c.
Let me know if this would prevent getting your ack for the patch :).
> >
> > static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
> > @@ -899,43 +964,17 @@ int tls_sw_recvmsg(struct sock *sk,
> > }
> >
> > if (!ctx->decrypted) {
> > - int page_count;
> > - int to_copy;
^ permalink raw reply
* Re: [PATCH RFC net-next 1/1] net/tls: Combined memory allocation for decryption request
From: Dave Watson @ 2018-08-07 14:56 UTC (permalink / raw)
To: Vakul Garg; +Cc: netdev, borisp, aviadye, davem
In-Reply-To: <20180806163810.30880-2-vakul.garg@nxp.com>
Hi Vakul,
Only minor comments, mostly looks good to me. Thanks
> +/* This function decrypts the input skb into either out_iov or in out_sg
> + * or in skb buffers itself. The input parameter 'zc' indicates if
> + * zero-copy mode needs to be tried or not. With zero-copy mode, either
> + * out_iov or out_sg must be non-NULL. In case both out_iov and out_sg are
> + * NULL, then the decryption happens inside skb buffers itself, i.e.
> + * zero-copy gets disabled and 'zc' is updated.
> + */
> +
> +static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
> + struct iov_iter *out_iov,
> + struct scatterlist *out_sg,
> + int *chunk, bool *zc)
> +{
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> + struct strp_msg *rxm = strp_msg(skb);
> + int n_sgin, n_sgout, nsg, mem_size, aead_size, err, pages = 0;
> + struct aead_request *aead_req;
> + struct sk_buff *unused;
> + u8 *aad, *iv, *mem = NULL;
> + struct scatterlist *sgin = NULL;
> + struct scatterlist *sgout = NULL;
> + const int data_len = rxm->full_len - tls_ctx->rx.overhead_size;
> +
> + if (*zc && (out_iov || out_sg)) {
> + if (out_iov)
> + n_sgout = iov_iter_npages(out_iov, INT_MAX) + 1;
> + else if (out_sg)
> + n_sgout = sg_nents(out_sg);
> + else
> + goto no_zerocopy;
Is the last else necessary? It looks like the if already checks for
out_iov || out_sg.
> struct scatterlist *sgout)
> {
> - struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> - char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + MAX_IV_SIZE];
> - struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
> - struct scatterlist *sgin = &sgin_arr[0];
> - struct strp_msg *rxm = strp_msg(skb);
> - int ret, nsg;
> - struct sk_buff *unused;
> -
> - ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
> - iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> - tls_ctx->rx.iv_size);
> - if (ret < 0)
> - return ret;
> -
> - memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> - if (!sgout) {
> - nsg = skb_cow_data(skb, 0, &unused);
> - } else {
> - nsg = skb_nsg(skb,
> - rxm->offset + tls_ctx->rx.prepend_size,
> - rxm->full_len - tls_ctx->rx.prepend_size);
> - if (nsg <= 0)
> - return nsg;
> - }
> -
> - // We need one extra for ctx->rx_aad_ciphertext
> - nsg++;
> -
> - if (nsg > ARRAY_SIZE(sgin_arr))
> - sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
> -
> - if (!sgout)
> - sgout = sgin;
> -
> - sg_init_table(sgin, nsg);
> - sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
> -
> - nsg = skb_to_sgvec(skb, &sgin[1],
> - rxm->offset + tls_ctx->rx.prepend_size,
> - rxm->full_len - tls_ctx->rx.prepend_size);
> - if (nsg < 0) {
> - ret = nsg;
> - goto out;
> - }
> -
> - tls_make_aad(ctx->rx_aad_ciphertext,
> - rxm->full_len - tls_ctx->rx.overhead_size,
> - tls_ctx->rx.rec_seq,
> - tls_ctx->rx.rec_seq_size,
> - ctx->control);
> -
> - ret = tls_do_decryption(sk, sgin, sgout, iv,
> - rxm->full_len - tls_ctx->rx.overhead_size,
> - skb, sk->sk_allocation);
> -
> -out:
> - if (sgin != &sgin_arr[0])
> - kfree(sgin);
> + bool zc = true;
> + int chunk;
>
> - return ret;
> + return decrypt_internal(sk, skb, NULL, sgout, &chunk, &zc);
> }
Can we merge this function to callsites? It's pretty useless now.
>
> static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
> @@ -899,43 +964,17 @@ int tls_sw_recvmsg(struct sock *sk,
> }
>
> if (!ctx->decrypted) {
> - int page_count;
> - int to_copy;
^ permalink raw reply
* Re: [PATCH net-next] RDS: IB: fix 'passing zero to ERR_PTR()' warning
From: Santosh Shilimkar @ 2018-08-07 17:09 UTC (permalink / raw)
To: YueHaibing, davem; +Cc: linux-kernel, netdev, linux-rdma, rds-devel
In-Reply-To: <20180807113416.12680-1-yuehaibing@huawei.com>
On 8/7/2018 4:34 AM, YueHaibing wrote:
> Fix a static code checker warning:
> net/rds/ib_frmr.c:82 rds_ib_alloc_frmr() warn: passing zero to 'ERR_PTR'
>
> The error path for ib_alloc_mr failure should set err to PTR_ERR.
>
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
Looks good. Thanks !!
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: [PATCH 3/3] net: phy: Add support for Broadcom Omega internal Combo GPHY
From: Florian Fainelli @ 2018-08-07 17:06 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Vivien Didelot, Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list
In-Reply-To: <1533661364-15381-4-git-send-email-arun.parameswaran@broadcom.com>
On 08/07/2018 10:02 AM, Arun Parameswaran wrote:
> Add support for the Broadcom Omega SoC internal Combo Ethernet
> GPHY to the bcm7xxx phy driver.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 2/3] net: dsa: b53: Add support for Broadcom Omega SoC internal switch
From: Florian Fainelli @ 2018-08-07 17:06 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Vivien Didelot, Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list
In-Reply-To: <1533661364-15381-3-git-send-email-arun.parameswaran@broadcom.com>
On 08/07/2018 10:02 AM, Arun Parameswaran wrote:
> Add support for the Broadcom Omega SoC internal ethernet switch
> to the b53 srab driver in the DSA framework.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: net: dsa: Add compatibility strings for Broadcom Omega
From: Florian Fainelli @ 2018-08-07 17:06 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Vivien Didelot, Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list
In-Reply-To: <1533661364-15381-2-git-send-email-arun.parameswaran@broadcom.com>
On 08/07/2018 10:02 AM, Arun Parameswaran wrote:
> Add compatibility strings for the internal switch in the Broadcom
> Omega SoC family (BCM5831X/BCM1140X) to B53.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v4] can: sja1000: plx_pci: add support for ASEM CAN raw device
From: Flavio Suligoi @ 2018-08-07 14:50 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger
Cc: David S . Miller, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <e5e06710-0304-0cf9-67fb-1e29f0678c86@pengutronix.de>
On 08/07/2018 04:11 PM, Marc Kleine-Budde wrote:
>On 08/07/2018 09:17 AM, Flavio Suligoi wrote:
>> This patch adds support for ASEM opto-isolated dual channels
>> CAN raw device (http://www.asem.it)
>>
>> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
>> ---
>>
>> v2: - rename ASEM_... constants to reduce space size;
>> - remove "else" in "plx_pci_reset_asem_dual_can_raw" function to avoid
>> strings breaking
>> v3: - fix wrong comment for PLX_LINT2_POL
>> - put string into just one line in "plx_pci_reset_asem_dual_can_raw"
>> function
>> v4: - remove unnecessary variable "reset_bar" in
>> "plx_pci_reset_asem_dual_can_raw" function
>>
>> drivers/net/can/sja1000/Kconfig | 1 +
>> drivers/net/can/sja1000/plx_pci.c | 66 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
>> index 1e65cb6..f6dc899 100644
>> --- a/drivers/net/can/sja1000/Kconfig
>> +++ b/drivers/net/can/sja1000/Kconfig
>> @@ -88,6 +88,7 @@ config CAN_PLX_PCI
>> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
>> - IXXAT Automation PC-I 04/PCI card (http://www.ixxat.com/)
>> - Connect Tech Inc. CANpro/104-Plus Opto (CRG001) card (http://www.connecttech.com)
>> + - ASEM CAN raw - 2 isolated CAN channels (www.asem.it)
>>
>> config CAN_TSCAN1
>> tristate "TS-CAN1 PC104 boards"
>> diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
>> index f8ff25c..79fc327 100644
>> --- a/drivers/net/can/sja1000/plx_pci.c
>> +++ b/drivers/net/can/sja1000/plx_pci.c
>> @@ -46,7 +46,8 @@ MODULE_SUPPORTED_DEVICE("Adlink PCI-7841/cPCI-7841, "
>> "esd CAN-PCIe/2000, "
>> "Connect Tech Inc. CANpro/104-Plus Opto (CRG001), "
>> "IXXAT PC-I 04/PCI, "
>> - "ELCUS CAN-200-PCI")
>> + "ELCUS CAN-200-PCI, "
>> + "ASEM DUAL CAN-RAW")
>> MODULE_LICENSE("GPL v2");
>>
>> #define PLX_PCI_MAX_CHAN 2
>> @@ -70,7 +71,9 @@ struct plx_pci_card {
>> */
>>
>> #define PLX_LINT1_EN 0x1 /* Local interrupt 1 enable */
>> +#define PLX_LINT1_POL (1 << 1) /* Local interrupt 1 polarity */
>> #define PLX_LINT2_EN (1 << 3) /* Local interrupt 2 enable */
>> +#define PLX_LINT2_POL (1 << 4) /* Local interrupt 2 polarity */
>> #define PLX_PCI_INT_EN (1 << 6) /* PCI Interrupt Enable */
>> #define PLX_PCI_RESET (1 << 30) /* PCI Adapter Software Reset */
>>
>> @@ -92,6 +95,9 @@ struct plx_pci_card {
>> */
>> #define PLX_PCI_OCR (OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL)
>>
>> +/* OCR setting for ASEM Dual CAN raw */
>> +#define ASEM_PCI_OCR 0xfe
>> +
>> /*
>> * In the CDR register, you should set CBP to 1.
>> * You will probably also want to set the clock divider value to 7
>> @@ -145,10 +151,20 @@ struct plx_pci_card {
>> #define MOXA_PCI_VENDOR_ID 0x1393
>> #define MOXA_PCI_DEVICE_ID 0x0100
>>
>> +#define ASEM_RAW_CAN_VENDOR_ID 0x10b5
>> +#define ASEM_RAW_CAN_DEVICE_ID 0x9030
>> +#define ASEM_RAW_CAN_SUB_VENDOR_ID 0x3000
>> +#define ASEM_RAW_CAN_SUB_DEVICE_ID 0x1001
>> +#define ASEM_RAW_CAN_SUB_DEVICE_ID_BIS 0x1002
>> +#define ASEM_RAW_CAN_RST_REGISTER 0x54
>> +#define ASEM_RAW_CAN_RST_MASK_CAN1 0x20
>> +#define ASEM_RAW_CAN_RST_MASK_CAN2 0x04
>> +
>> static void plx_pci_reset_common(struct pci_dev *pdev);
>> static void plx9056_pci_reset_common(struct pci_dev *pdev);
>> static void plx_pci_reset_marathon_pci(struct pci_dev *pdev);
>> static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev);
>> +static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev);
>>
>> struct plx_pci_channel_map {
>> u32 bar;
>> @@ -269,6 +285,14 @@ static struct plx_pci_card_info plx_pci_card_info_moxa = {
>> /* based on PLX9052 */
>> };
>>
>> +static struct plx_pci_card_info plx_pci_card_info_asem_dual_can = {
>> + "ASEM Dual CAN raw PCI", 2,
>> + PLX_PCI_CAN_CLOCK, ASEM_PCI_OCR, PLX_PCI_CDR,
>> + {0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
>> + &plx_pci_reset_asem_dual_can_raw
>> + /* based on PLX9030 */
>> +};
>> +
>> static const struct pci_device_id plx_pci_tbl[] = {
>> {
>> /* Adlink PCI-7841/cPCI-7841 */
>> @@ -375,6 +399,20 @@ static const struct pci_device_id plx_pci_tbl[] = {
>> 0, 0,
>> (kernel_ulong_t)&plx_pci_card_info_moxa
>> },
>> + {
>> + /* ASEM Dual CAN raw */
>> + ASEM_RAW_CAN_VENDOR_ID, ASEM_RAW_CAN_DEVICE_ID,
>> + ASEM_RAW_CAN_SUB_VENDOR_ID, ASEM_RAW_CAN_SUB_DEVICE_ID,
>> + 0, 0,
>> + (kernel_ulong_t)&plx_pci_card_info_asem_dual_can
>> + },
>> + {
>> + /* ASEM Dual CAN raw -new model */
>> + ASEM_RAW_CAN_VENDOR_ID, ASEM_RAW_CAN_DEVICE_ID,
>> + ASEM_RAW_CAN_SUB_VENDOR_ID, ASEM_RAW_CAN_SUB_DEVICE_ID_BIS,
>> + 0, 0,
>> + (kernel_ulong_t)&plx_pci_card_info_asem_dual_can
>> + },
>> { 0,}
>> };
>> MODULE_DEVICE_TABLE(pci, plx_pci_tbl);
>> @@ -524,6 +562,32 @@ static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev)
>> }
>> }
>>
>> +/* Special reset function for ASEM Dual CAN raw card */
>> +static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev)
>> +{
>> + void __iomem *bar0_addr;
>> + u8 tmpval;
>> +
>> + plx_pci_reset_common(pdev);
>> +
>> + bar0_addr = pci_iomap(pdev, 0, 0);
>> + if (!bar0_addr) {
>> + dev_err(&pdev->dev, "Failed to remap reset space %d (BAR%d)\n",
>> + 0, 0);
>
>I've put both 0 directly into the string while applying the patch.
>
>Tnx,
>Marc
Thanks,
Flavio
>
>--
>Pengutronix e.K. | Marc Kleine-Budde |
>Industrial Linux Solutions | Phone: +49-231-2826-924 |
>Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
>Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
^ permalink raw reply
* Re: [PATCH 1/3] net:svc_rdma_transport: remove unneeded variable 'ret' in rdma_listen_handler
From: Anna Schumaker @ 2018-08-07 14:49 UTC (permalink / raw)
To: zhong jiang, davem, J. Bruce Fields; +Cc: netdev, linux-kernel
In-Reply-To: <1533640809-1221-2-git-send-email-zhongjiang@huawei.com>
(Adding Bruce since he takes nfs / sunrpc server patches)
On 08/07/2018 07:20 AM, zhong jiang wrote:
> The ret is not modified after initalization, So just remove the variable
> and return 0.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index 547b2cd..2848caf 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -296,7 +296,6 @@ static int rdma_listen_handler(struct rdma_cm_id *cma_id,
> struct rdma_cm_event *event)
> {
> struct sockaddr *sap = (struct sockaddr *)&cma_id->route.addr.src_addr;
> - int ret = 0;
>
> trace_svcrdma_cm_event(event, sap);
>
> @@ -315,7 +314,7 @@ static int rdma_listen_handler(struct rdma_cm_id *cma_id,
> break;
> }
>
> - return ret;
> + return 0;
> }
>
> static int rdma_cma_handler(struct rdma_cm_id *cma_id,
>
^ permalink raw reply
* [PATCH 3/3] net: phy: Add support for Broadcom Omega internal Combo GPHY
From: Arun Parameswaran @ 2018-08-07 17:02 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Vivien Didelot,
Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list,
Arun Parameswaran
In-Reply-To: <1533661364-15381-1-git-send-email-arun.parameswaran@broadcom.com>
Add support for the Broadcom Omega SoC internal Combo Ethernet
GPHY to the bcm7xxx phy driver.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
drivers/net/phy/bcm7xxx.c | 2 ++
include/linux/brcmphy.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index 01d2ff2f..b2b6307 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -229,6 +229,7 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
phy_read(phydev, MII_BMSR);
switch (rev) {
+ case 0xa0:
case 0xb0:
ret = bcm7xxx_28nm_b0_afe_config_init(phydev);
break;
@@ -659,6 +660,7 @@ static int bcm7xxx_28nm_probe(struct phy_device *phydev)
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439, "Broadcom BCM7439"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7439_2, "Broadcom BCM7439 (2)"),
BCM7XXX_28NM_GPHY(PHY_ID_BCM7445, "Broadcom BCM7445"),
+ BCM7XXX_28NM_GPHY(PHY_ID_BCM_OMEGA, "Broadcom Omega Combo GPHY"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7346, "Broadcom BCM7346"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7362, "Broadcom BCM7362"),
BCM7XXX_40NM_EPHY(PHY_ID_BCM7425, "Broadcom BCM7425"),
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index daa9234..949e9af 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -45,6 +45,7 @@
#define PHY_ID_BCM7445 0x600d8510
#define PHY_ID_BCM_CYGNUS 0xae025200
+#define PHY_ID_BCM_OMEGA 0xae025100
#define PHY_BCM_OUI_MASK 0xfffffc00
#define PHY_BCM_OUI_1 0x00206000
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] dt-bindings: net: dsa: Add compatibility strings for Broadcom Omega
From: Arun Parameswaran @ 2018-08-07 17:02 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Vivien Didelot,
Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list,
Arun Parameswaran
In-Reply-To: <1533661364-15381-1-git-send-email-arun.parameswaran@broadcom.com>
Add compatibility strings for the internal switch in the Broadcom
Omega SoC family (BCM5831X/BCM1140X) to B53.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
Documentation/devicetree/bindings/net/dsa/b53.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/b53.txt b/Documentation/devicetree/bindings/net/dsa/b53.txt
index 47a6a7f..1811e19 100644
--- a/Documentation/devicetree/bindings/net/dsa/b53.txt
+++ b/Documentation/devicetree/bindings/net/dsa/b53.txt
@@ -24,6 +24,14 @@ Required properties:
"brcm,bcm53018-srab"
"brcm,bcm53019-srab" and the mandatory "brcm,bcm5301x-srab" string
+ For the BCM5831X/BCM1140x SoCs with an integrated switch, must be one of:
+ "brcm,bcm11404-srab"
+ "brcm,bcm11407-srab"
+ "brcm,bcm11409-srab"
+ "brcm,bcm58310-srab"
+ "brcm,bcm58311-srab"
+ "brcm,bcm58313-srab" and the mandatory "brcm,omega-srab" string
+
For the BCM585xx/586XX/88312 SoCs with an integrated switch, must be one of:
"brcm,bcm58522-srab"
"brcm,bcm58523-srab"
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] Add Broadcom Omega SoC internal switch and phy
From: Arun Parameswaran @ 2018-08-07 17:02 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, Andrew Lunn, Vivien Didelot,
Rob Herring, Mark Rutland
Cc: netdev, devicetree, linux-kernel, bcm-kernel-feedback-list,
Arun Parameswaran
Hi,
The patchset is based on David Miller's "net-next" repo.
The patches add support for the Broadcom Omega SoC's internal ethernet
switch and the internal gphy.
The internal ethernet switch in the Omega is a b53 srab based switch.
The support for the switch is added to the b53 driver in the dsa
framework.
The gphy support is added to the bcm7xxx driver.
Thanks
Arun
Arun Parameswaran (3):
dt-bindings: net: dsa: Add compatibility strings for Broadcom Omega
net: dsa: b53: Add support for Broadcom Omega SoC internal switch
net: phy: Add support for Broadcom Omega internal Combo GPHY
Documentation/devicetree/bindings/net/dsa/b53.txt | 8 ++++++++
drivers/net/dsa/b53/b53_srab.c | 1 +
drivers/net/phy/bcm7xxx.c | 2 ++
include/linux/brcmphy.h | 1 +
4 files changed, 12 insertions(+)
--
1.9.1
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] bpf: add bpf queue map
From: Alexei Starovoitov @ 2018-08-07 14:42 UTC (permalink / raw)
To: Mauricio Vasquez B; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <153356390770.6981.4228793745105954649.stgit@kernel>
On Mon, Aug 06, 2018 at 03:58:30PM +0200, Mauricio Vasquez B wrote:
> Bpf queue implements a LIFO/FIFO data containers for ebpf programs.
queue/stack datastructure would be a great addition.
> It allows to push an element to the queue by using the update operation
> and to pop an element from the queue by using the lookup operation.
>
> A use case for this is to keep track of a pool of elements, like
> network ports in a SNAT.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> include/linux/bpf_types.h | 1
> include/uapi/linux/bpf.h | 5 +
> kernel/bpf/Makefile | 2
> kernel/bpf/queuemap.c | 287 +++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 61 +++++++---
> kernel/bpf/verifier.c | 16 ++-
> 6 files changed, 353 insertions(+), 19 deletions(-)
> create mode 100644 kernel/bpf/queuemap.c
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index c5700c2d5549..6c7a62f3fe43 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -58,3 +58,4 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_CPUMAP, cpu_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_XSKMAP, xsk_map_ops)
> #endif
> #endif
> +BPF_MAP_TYPE(BPF_MAP_TYPE_QUEUE, queue_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0ebaaf7f3568..2c171c40eb45 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -120,6 +120,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_CPUMAP,
> BPF_MAP_TYPE_XSKMAP,
> BPF_MAP_TYPE_SOCKHASH,
> + BPF_MAP_TYPE_QUEUE,
> };
>
> enum bpf_prog_type {
> @@ -255,6 +256,10 @@ enum bpf_attach_type {
> /* Flag for stack_map, store build_id+offset instead of pointer */
> #define BPF_F_STACK_BUILD_ID (1U << 5)
>
> +/* Flags for queue_map, type of queue */
> +#define BPF_F_QUEUE_FIFO (1U << 16)
> +#define BPF_F_QUEUE_LIFO (2U << 16)
the choice of flags looks odd.
Why start at bit 16 and why waste two bits?
It's either stack or queue.
May be instead define two map_types:
BPF_MAP_TYPE_QUEUE and BPF_MAP_TYPE_STACK
that share common implementation?
And how about adding three new helpers: push/pop/peek as well?
Reusing lookup/update is neat, but does lookup == pop
or does lookup == peek ?
I suspect it will be confusing.
Three new helpers cost nothing, but will make bpf progs easier to read.
Could you also add a flag for replacement policy?
In this implementation when max_entries limit is reached
the map_update_elem (aka push) will fail with e2big.
It would be useful to allow pushing and dropping elements from
the other side then such queue/stack can be used to keep
track of the last N events (the prog would unconditionally push
and user space would swap the queue for new one via map-in-map
and drain old queue).
Speaking of map-in-map, please add a test to make sure
queue/stack works with hash/array_of_maps.
selftests in patch2 needs to have kernel side as well.
it's not enough to test syscall access only.
^ permalink raw reply
* [PATCH net-next] ieee802154: hwsim: fix rcu address annotation
From: Alexander Aring @ 2018-08-07 14:34 UTC (permalink / raw)
To: netdev; +Cc: linux-wpan, kernel, Alexander Aring, Stefan Schmidt
This patch fixes the following sparse warning about mismatch rcu
attribute for address space annotation:
...
error: incompatible types in comparison expression (different modifiers)
error: incompatible types in comparison expression (different address spaces)
...
Some __rcu annotation was at non-pointers list head structures and one was
missing in edge information which is used by rcu_assign_pointer() to
update edge setting information.
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
Hopefully I did it right now. Sorry again.
drivers/net/ieee802154/mac802154_hwsim.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 1982308b9b1c..f4e92054f7df 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -36,7 +36,7 @@ MODULE_LICENSE("GPL");
static LIST_HEAD(hwsim_phys);
static DEFINE_MUTEX(hwsim_phys_lock);
-static __rcu LIST_HEAD(hwsim_ifup_phys);
+static LIST_HEAD(hwsim_ifup_phys);
static struct platform_device *mac802154hwsim_dev;
@@ -68,7 +68,7 @@ struct hwsim_edge_info {
struct hwsim_edge {
struct hwsim_phy *endpoint;
- struct hwsim_edge_info *info;
+ struct hwsim_edge_info __rcu *info;
struct list_head list;
struct rcu_head rcu;
@@ -81,7 +81,7 @@ struct hwsim_phy {
struct hwsim_pib __rcu *pib;
bool suspended;
- struct list_head __rcu edges;
+ struct list_head edges;
struct list_head list;
struct list_head list_ifup;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v8 bpf-next 02/10] veth: Add driver XDP
From: Daniel Borkmann @ 2018-08-07 14:26 UTC (permalink / raw)
To: Toshiaki Makita, Alexei Starovoitov
Cc: netdev, Jesper Dangaard Brouer, Jakub Kicinski, John Fastabend
In-Reply-To: <1533283098-2397-3-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 08/03/2018 09:58 AM, Toshiaki Makita wrote:
> This is the basic implementation of veth driver XDP.
>
> Incoming packets are sent from the peer veth device in the form of skb,
> so this is generally doing the same thing as generic XDP.
>
> This itself is not so useful, but a starting point to implement other
> useful veth XDP features like TX and REDIRECT.
>
> This introduces NAPI when XDP is enabled, because XDP is now heavily
> relies on NAPI context. Use ptr_ring to emulate NIC ring. Tx function
> enqueues packets to the ring and peer NAPI handler drains the ring.
>
> Currently only one ring is allocated for each veth device, so it does
> not scale on multiqueue env. This can be resolved by allocating rings
> on the per-queue basis later.
>
> Note that NAPI is not used but netif_rx is used when XDP is not loaded,
> so this does not change the default behaviour.
>
> v6:
> - Check skb->len only when allocation is needed.
> - Add __GFP_NOWARN to alloc_page() as it can be triggered by external
> events.
>
> v3:
> - Fix race on closing the device.
> - Add extack messages in ndo_bpf.
>
> v2:
> - Squashed with the patch adding NAPI.
> - Implement adjust_tail.
> - Don't acquire consumer lock because it is guarded by NAPI.
> - Make poll_controller noop since it is unnecessary.
> - Register rxq_info on enabling XDP rather than on opening the device.
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
[...]
> +
> +static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
> + struct sk_buff *skb)
> +{
> + u32 pktlen, headroom, act, metalen;
> + void *orig_data, *orig_data_end;
> + struct bpf_prog *xdp_prog;
> + int mac_len, delta, off;
> + struct xdp_buff xdp;
> +
> + rcu_read_lock();
> + xdp_prog = rcu_dereference(priv->xdp_prog);
> + if (unlikely(!xdp_prog)) {
> + rcu_read_unlock();
> + goto out;
> + }
> +
> + mac_len = skb->data - skb_mac_header(skb);
> + pktlen = skb->len + mac_len;
> + headroom = skb_headroom(skb) - mac_len;
> +
> + if (skb_shared(skb) || skb_head_is_locked(skb) ||
> + skb_is_nonlinear(skb) || headroom < XDP_PACKET_HEADROOM) {
Hmm, I think this is not fully correct. What happens if you have cloned skbs as
e.g. the case with TCP? This would also need a full expensive unclone to make the
data private as expected by XDP (this is basically a similar issue in generic
XDP). It may potentially be worth to also share the code here with generic XDP
implementation given it's quite similar?
> + struct sk_buff *nskb;
> + int size, head_off;
> + void *head, *start;
> + struct page *page;
> +
> + size = SKB_DATA_ALIGN(VETH_XDP_HEADROOM + pktlen) +
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> + if (size > PAGE_SIZE)
> + goto drop;
> +
> + page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
> + if (!page)
> + goto drop;
> +
> + head = page_address(page);
> + start = head + VETH_XDP_HEADROOM;
> + if (skb_copy_bits(skb, -mac_len, start, pktlen)) {
> + page_frag_free(head);
> + goto drop;
> + }
> +
> + nskb = veth_build_skb(head,
> + VETH_XDP_HEADROOM + mac_len, skb->len,
> + PAGE_SIZE);
> + if (!nskb) {
> + page_frag_free(head);
> + goto drop;
> + }
> +
> + skb_copy_header(nskb, skb);
> + head_off = skb_headroom(nskb) - skb_headroom(skb);
> + skb_headers_offset_update(nskb, head_off);
> + if (skb->sk)
> + skb_set_owner_w(nskb, skb->sk);
> + consume_skb(skb);
> + skb = nskb;
> + }
> +
> + xdp.data_hard_start = skb->head;
> + xdp.data = skb_mac_header(skb);
> + xdp.data_end = xdp.data + pktlen;
> + xdp.data_meta = xdp.data;
> + xdp.rxq = &priv->xdp_rxq;
> + orig_data = xdp.data;
> + orig_data_end = xdp.data_end;
> +
> + act = bpf_prog_run_xdp(xdp_prog, &xdp);
> +
> + switch (act) {
> + case XDP_PASS:
> + break;
> + default:
> + bpf_warn_invalid_xdp_action(act);
> + case XDP_ABORTED:
> + trace_xdp_exception(priv->dev, xdp_prog, act);
> + case XDP_DROP:
> + goto drop;
> + }
> + rcu_read_unlock();
> +
> + delta = orig_data - xdp.data;
> + off = mac_len + delta;
> + if (off > 0)
> + __skb_push(skb, off);
> + else if (off < 0)
> + __skb_pull(skb, -off);
> + skb->mac_header -= delta;
> + off = xdp.data_end - orig_data_end;
> + if (off != 0)
> + __skb_put(skb, off);
> + skb->protocol = eth_type_trans(skb, priv->dev);
> +
> + metalen = xdp.data - xdp.data_meta;
> + if (metalen)
> + skb_metadata_set(skb, metalen);
> +out:
> + return skb;
> +drop:
> + rcu_read_unlock();
> + kfree_skb(skb);
> + return NULL;
> +}
^ permalink raw reply
* Re: [net-next:master 1745/1753] drivers/net/ieee802154/mac802154_hwsim.c:39:14: sparse: incorrect type in initializer (different address spaces)
From: Alexander Aring @ 2018-08-07 13:54 UTC (permalink / raw)
To: netdev; +Cc: Stefan Schmidt
In-Reply-To: <201808070717.QVq6ok1n%fengguang.wu@intel.com>
Hi,
On Tue, Aug 07, 2018 at 07:07:20AM +0800, kbuild test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> head: de7de576eca204de9a38e2f6dafe6b7c1ddc85c1
> commit: f25da51fdc381ca2863248c7060b3662632f0872 [1745/1753] ieee802154: hwsim: add replacement for fakelb
> reproduce:
> # apt-get install sparse
> git checkout f25da51fdc381ca2863248c7060b3662632f0872
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
oops sorry, I will take care of those immediately.
- Alex
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] bpf: add bpf queue map
From: Daniel Borkmann @ 2018-08-07 13:52 UTC (permalink / raw)
To: Mauricio Vasquez B, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <153356390770.6981.4228793745105954649.stgit@kernel>
On 08/06/2018 03:58 PM, Mauricio Vasquez B wrote:
> Bpf queue implements a LIFO/FIFO data containers for ebpf programs.
>
> It allows to push an element to the queue by using the update operation
> and to pop an element from the queue by using the lookup operation.
>
> A use case for this is to keep track of a pool of elements, like
> network ports in a SNAT.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
[...]
> +static int prealloc_init(struct bpf_queue *queue)
> +{
> + u32 node_size = sizeof(struct queue_node) +
> + round_up(queue->map.value_size, 8);
> + u32 num_entries = queue->map.max_entries;
> + int err;
> +
> + queue->nodes = bpf_map_area_alloc(node_size * num_entries,
> + queue->map.numa_node);
That doesn't work either. If you don't set numa node, then here in
your case you'll always use numa node 0, which is unintentional.
You need to get the node via bpf_map_attr_numa_node(attr) helper.
Same issue in queue_map_update_elem().
^ permalink raw reply
* for editing
From: Roy Robinson @ 2018-08-07 10:51 UTC (permalink / raw)
To: netdev
Did you receive my email yesterday?
Just want to check if you have needs for photo editing for our studio?
We normally edit 300 images within 12-24 hours.
We take care of different kinds of e-commerce photos, jewelry images, and
portrait model images.
Including cutting out and clipping path and others, we also give retouching
for your images.
You may drop us a photo if you want to check our quality, we will provide
testing.
Thanks,
Roy
^ permalink raw reply
* Re: [PATCH bpf-next 3/3] bpf: add sample for BPF_MAP_TYPE_QUEUE
From: Daniel Borkmann @ 2018-08-07 13:44 UTC (permalink / raw)
To: Mauricio Vasquez B, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <153356392410.6981.1290059578982921349.stgit@kernel>
On 08/06/2018 03:58 PM, Mauricio Vasquez B wrote:
> The example is made by two parts, a eBPF program that consumes elements
> from a FIFO queue and prints them in the screen and a user space
> application that inserts new elements into the queue each time this is
> executed.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> samples/bpf/.gitignore | 1 +
> samples/bpf/Makefile | 3 ++
> samples/bpf/test_map_in_map_user.c | 9 +-----
> samples/bpf/test_queuemap.sh | 37 +++++++++++++++++++++++++
> samples/bpf/test_queuemap_kern.c | 51 +++++++++++++++++++++++++++++++++++
> samples/bpf/test_queuemap_user.c | 53 ++++++++++++++++++++++++++++++++++++
> 6 files changed, 147 insertions(+), 7 deletions(-)
> create mode 100755 samples/bpf/test_queuemap.sh
> create mode 100644 samples/bpf/test_queuemap_kern.c
> create mode 100644 samples/bpf/test_queuemap_user.c
>
> diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
> index 8ae4940025f8..d7e518c1b3ed 100644
> --- a/samples/bpf/.gitignore
> +++ b/samples/bpf/.gitignore
> @@ -26,6 +26,7 @@ test_lru_dist
> test_map_in_map
> test_overhead
> test_probe_write_user
> +test_queuemap
> trace_event
> trace_output
> tracex1
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index f88d5683d6ee..624f4f4b81db 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -53,6 +53,7 @@ hostprogs-y += xdpsock
> hostprogs-y += xdp_fwd
> hostprogs-y += task_fd_query
> hostprogs-y += xdp_sample_pkts
> +hostprogs-y += test_queuemap
>
> # Libbpf dependencies
> LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
> @@ -109,6 +110,7 @@ xdpsock-objs := xdpsock_user.o
> xdp_fwd-objs := xdp_fwd_user.o
> task_fd_query-objs := bpf_load.o task_fd_query_user.o $(TRACE_HELPERS)
> xdp_sample_pkts-objs := xdp_sample_pkts_user.o $(TRACE_HELPERS)
> +test_queuemap-objs := bpf_load.o test_queuemap_user.o
>
> # Tell kbuild to always build the programs
> always := $(hostprogs-y)
> @@ -166,6 +168,7 @@ always += xdpsock_kern.o
> always += xdp_fwd_kern.o
> always += task_fd_query_kern.o
> always += xdp_sample_pkts_kern.o
> +always += test_queuemap_kern.o
>
> HOSTCFLAGS += -I$(objtree)/usr/include
> HOSTCFLAGS += -I$(srctree)/tools/lib/
> diff --git a/samples/bpf/test_map_in_map_user.c b/samples/bpf/test_map_in_map_user.c
> index e308858f7bcf..28edac94234e 100644
> --- a/samples/bpf/test_map_in_map_user.c
> +++ b/samples/bpf/test_map_in_map_user.c
> @@ -1,10 +1,5 @@
> -/*
> - * Copyright (c) 2017 Facebook
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of version 2 of the GNU General Public
> - * License as published by the Free Software Foundation.
> - */
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018 Politecnico di Torino */
I don't think you can remove above Copyright and replace it with a different one. ;)
> #include <sys/resource.h>
> #include <sys/socket.h>
> #include <arpa/inet.h>
> diff --git a/samples/bpf/test_queuemap.sh b/samples/bpf/test_queuemap.sh
> new file mode 100755
> index 000000000000..ed08c1fa8c2c
> --- /dev/null
> +++ b/samples/bpf/test_queuemap.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +[[ -z $TC ]] && TC='tc'
> +[[ -z $IP ]] && IP='ip'
> +
> +TEST_QUEUE_USER='./test_queuemap'
> +TEST_QUEUE_BPF='./test_queuemap_kern.o'
> +
> +function config {
> + $IP netns add ns1
> + $IP link add ve1 type veth peer name vens1
> + $IP link set dev ve1 up
> + $IP link set dev ve1 mtu 1500
> + $IP link set dev vens1 netns ns1
> +
> + $IP -n ns1 link set dev lo up
> + $IP -n ns1 link set dev vens1 up
> + $IP -n ns1 addr add 10.1.1.101/24 dev vens1
> +
> + $IP addr add 10.1.1.1/24 dev ve1
> + $TC qdisc add dev ve1 clsact
> + $TC filter add dev ve1 ingress bpf da obj $TEST_QUEUE_BPF sec test_queue
> +}
> +
> +function cleanup {
> + set +e
> + [[ -z $DEBUG ]] || set +x
> + $IP netns delete ns1 >& /dev/null
> + $IP link del ve1 >& /dev/null
> + rm -f /sys/fs/bpf/tc/globals/queue
> + [[ -z $DEBUG ]] || set -x
> + set -e
> +}
> +
> +cleanup
> +config
> diff --git a/samples/bpf/test_queuemap_kern.c b/samples/bpf/test_queuemap_kern.c
> new file mode 100644
> index 000000000000..2b496dafaffd
> --- /dev/null
> +++ b/samples/bpf/test_queuemap_kern.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018 Politecnico di Torino */
> +#define KBUILD_MODNAME "foo"
> +#include <linux/ptrace.h>
> +#include <linux/version.h>
> +#include <uapi/linux/bpf.h>
> +#include <uapi/linux/in6.h>
> +#include <uapi/linux/pkt_cls.h>
> +#include "bpf_helpers.h"
> +
> +#define PIN_GLOBAL_NS 2
> +
> +struct bpf_elf_map {
> + __u32 type;
> + __u32 key_size;
> + __u32 value_size;
> + __u32 max_entries;
> + __u32 flags;
> + __u32 id;
> + __u32 pinning;
> +};
> +
> +/* map #0 */
> +struct bpf_elf_map SEC("maps") queue = {
> + .type = BPF_MAP_TYPE_QUEUE,
> + .key_size = 0,
> + .value_size = sizeof(u32),
> + .flags = BPF_F_QUEUE_FIFO,
> + .max_entries = 1024,
> + .pinning = PIN_GLOBAL_NS,
> +};
> +
> +SEC("test_queue")
> +int _test_queue(struct __sk_buff *skb)
> +{
> + char msg[] = "element is %u\n";
> + char msg_no[] = "there are not elements\n";
> +
> + u32 *val = bpf_map_lookup_elem(&queue, NULL);
> +
> + if (!val) {
> + bpf_trace_printk(msg_no, sizeof(msg_no));
> + return TC_ACT_OK;
> + }
> +
> + bpf_trace_printk(msg, sizeof(msg), *val);
Could we add a more elaborate example? In cover letter and patch 1 which
implements the map, you mention SNAT, perhaps it would be useful to make
it a minimal sample app.
> + return TC_ACT_OK;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +u32 _version SEC("version") = LINUX_VERSION_CODE;
> diff --git a/samples/bpf/test_queuemap_user.c b/samples/bpf/test_queuemap_user.c
> new file mode 100644
> index 000000000000..68f9a5d54596
> --- /dev/null
> +++ b/samples/bpf/test_queuemap_user.c
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (c) 2018 Politecnico di Torino
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */
> +#include <sys/resource.h>
> +#include <sys/socket.h>
> +#include <arpa/inet.h>
> +#include <stdint.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <bpf/bpf.h>
> +#include "bpf_load.h"
> +
> +int queue_map;
> +
> +static void test_queue_map(void)
> +{
> + int ret;
> + uint32_t i;
> +
> + queue_map = bpf_obj_get("/sys/fs/bpf/tc/globals/queue");
> + if (queue_map < 0) {
> + fprintf(stderr, "error getting map");
> + return;
> + }
> +
> + for (i = 0; i < 256; i++) {
> + uint32_t v = 1000 - i*3;
> +
> + ret = bpf_map_update_elem(queue_map, NULL, &v, 0);
> + if (ret)
> + fprintf(stderr, "ret is %d\n", ret);
> + }
> +}
> +
> +int main(int argc, char **argv)
> +{
> + struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> + char filename[256];
> +
> + assert(!setrlimit(RLIMIT_MEMLOCK, &r));
> +
> + snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
> +
> + test_queue_map();
> +
> + return 0;
> +}
>
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] selftests/bpf: add test cases for BPF_MAP_TYPE_QUEUE
From: Daniel Borkmann @ 2018-08-07 13:42 UTC (permalink / raw)
To: Mauricio Vasquez B, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <153356391611.6981.14864460244240605372.stgit@kernel>
On 08/06/2018 03:58 PM, Mauricio Vasquez B wrote:
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> tools/include/uapi/linux/bpf.h | 5 ++
> tools/testing/selftests/bpf/test_maps.c | 72 +++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 0ebaaf7f3568..2c171c40eb45 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -120,6 +120,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_CPUMAP,
> BPF_MAP_TYPE_XSKMAP,
> BPF_MAP_TYPE_SOCKHASH,
> + BPF_MAP_TYPE_QUEUE,
> };
>
> enum bpf_prog_type {
> @@ -255,6 +256,10 @@ enum bpf_attach_type {
> /* Flag for stack_map, store build_id+offset instead of pointer */
> #define BPF_F_STACK_BUILD_ID (1U << 5)
>
> +/* Flags for queue_map, type of queue */
> +#define BPF_F_QUEUE_FIFO (1U << 16)
> +#define BPF_F_QUEUE_LIFO (2U << 16)
> +
> enum bpf_stack_build_id_status {
> /* user space need an empty entry to identify end of a trace */
> BPF_STACK_BUILD_ID_EMPTY = 0,
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index 6c253343a6f9..34567b017dbb 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -457,6 +457,77 @@ static void test_devmap(int task, void *data)
> close(fd);
> }
>
> +static void test_queuemap(int task, void *data)
> +{
> + __u32 value;
> + int fd, i;
> +
> + /* test FIFO */
> + fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, sizeof(value), 32,
> + BPF_F_QUEUE_FIFO);
We should also feed in 'map_flags' so that both prealloc and non-prealloc
will be tested.
> + if (fd < 0) {
> + printf("Failed to create queuemap '%s'!\n", strerror(errno));
> + exit(1);
> + }
> +
> + /* Push 32 elements */
> + for (i = 0; i < 32; i++) {
> + value = 1000 - i * 3;
> + assert(bpf_map_update_elem(fd, NULL, &value, 0) == 0);
> + }
> +
> + /* Check that element cannot be pushed due to max_entries limit */
> + value = 1000;
> + assert(bpf_map_update_elem(fd, NULL, &value, 0) == -1 &&
> + errno == E2BIG);
> +
> + /* Pop all elements */
> + for (i = 0; i < 32; i++)
> + assert(bpf_map_lookup_elem(fd, NULL, &value) == 0 &&
> + value == (1000 - i * 3));
> +
> + /* Check that there are not elements left */
> + assert(bpf_map_lookup_elem(fd, NULL, &value) == -1 && errno == ENOENT);
> +
> + assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> + assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> +
> + close(fd);
> +
> + /* test LIFO */
> + fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, sizeof(value), 32,
> + BPF_F_QUEUE_LIFO);
Ditto.
> + if (fd < 0) {
> + printf("Failed to create queuemap '%s'!\n", strerror(errno));
> + exit(1);
> + }
> +
> + /* Push 32 elements */
> + for (i = 0; i < 32; i++) {
> + value = 1000 - i * 3;
> + assert(bpf_map_update_elem(fd, NULL, &value, 0) == 0);
> + }
> +
> + /* Check that element cannot be pushed due to max_entries limit */
> + value = 1000;
> + assert(bpf_map_update_elem(fd, NULL, &value, 0) == -1 &&
> + errno == E2BIG);
> +
> + /* Pop all elements */
> + for (i = 31; i >= 0; i--)
> + assert(bpf_map_lookup_elem(fd, NULL, &value) == 0 &&
> + value == (1000 - i * 3));
> +
> + /* Check that there are not elements left */
> + assert(bpf_map_lookup_elem(fd, NULL, &value) == -1 &&
> + errno == ENOENT);
> +
> + assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> + assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> +
> + close(fd);
> +}
> +
> #include <sys/socket.h>
> #include <sys/ioctl.h>
> #include <arpa/inet.h>
> @@ -1162,6 +1233,7 @@ static void run_all_tests(void)
> test_arraymap_percpu_many_keys();
>
> test_devmap(0, NULL);
> + test_queuemap(0, NULL);
> test_sockmap(0, NULL);
>
> test_map_large();
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] bpf: add bpf queue map
From: Daniel Borkmann @ 2018-08-07 13:40 UTC (permalink / raw)
To: Mauricio Vasquez B, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <153356390770.6981.4228793745105954649.stgit@kernel>
On 08/06/2018 03:58 PM, Mauricio Vasquez B wrote:
> Bpf queue implements a LIFO/FIFO data containers for ebpf programs.
>
> It allows to push an element to the queue by using the update operation
> and to pop an element from the queue by using the lookup operation.
>
> A use case for this is to keep track of a pool of elements, like
> network ports in a SNAT.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> include/linux/bpf_types.h | 1
> include/uapi/linux/bpf.h | 5 +
> kernel/bpf/Makefile | 2
> kernel/bpf/queuemap.c | 287 +++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 61 +++++++---
> kernel/bpf/verifier.c | 16 ++-
> 6 files changed, 353 insertions(+), 19 deletions(-)
> create mode 100644 kernel/bpf/queuemap.c
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index c5700c2d5549..6c7a62f3fe43 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -58,3 +58,4 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_CPUMAP, cpu_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_XSKMAP, xsk_map_ops)
> #endif
> #endif
> +BPF_MAP_TYPE(BPF_MAP_TYPE_QUEUE, queue_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0ebaaf7f3568..2c171c40eb45 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -120,6 +120,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_CPUMAP,
> BPF_MAP_TYPE_XSKMAP,
> BPF_MAP_TYPE_SOCKHASH,
> + BPF_MAP_TYPE_QUEUE,
> };
>
> enum bpf_prog_type {
> @@ -255,6 +256,10 @@ enum bpf_attach_type {
> /* Flag for stack_map, store build_id+offset instead of pointer */
> #define BPF_F_STACK_BUILD_ID (1U << 5)
>
> +/* Flags for queue_map, type of queue */
> +#define BPF_F_QUEUE_FIFO (1U << 16)
> +#define BPF_F_QUEUE_LIFO (2U << 16)
> +
> enum bpf_stack_build_id_status {
> /* user space need an empty entry to identify end of a trace */
> BPF_STACK_BUILD_ID_EMPTY = 0,
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index f27f5496d6fe..30f02ef66635 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -2,7 +2,7 @@
> obj-y := core.o
>
> obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
> -obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
> +obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o queuemap.o
> obj-$(CONFIG_BPF_SYSCALL) += disasm.o
> obj-$(CONFIG_BPF_SYSCALL) += btf.o
> ifeq ($(CONFIG_NET),y)
> diff --git a/kernel/bpf/queuemap.c b/kernel/bpf/queuemap.c
> new file mode 100644
> index 000000000000..ab30af43b4cc
> --- /dev/null
> +++ b/kernel/bpf/queuemap.c
> @@ -0,0 +1,287 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * queuemap.c: BPF queue map
> + *
> + * Copyright (c) 2018 Politecnico di Torino
> + */
> +#include <linux/bpf.h>
> +#include <linux/rculist.h>
> +#include <linux/slab.h>
> +#include "percpu_freelist.h"
> +
> +#define QUEUE_CREATE_FLAG_MASK \
> + (BPF_F_NO_PREALLOC | BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY | \
> + BPF_F_QUEUE_FIFO | BPF_F_QUEUE_LIFO)
> +
> +enum queue_type {
> + QUEUE_FIFO = (BPF_F_QUEUE_FIFO >> 16),
> + QUEUE_LIFO = (BPF_F_QUEUE_LIFO >> 16),
> +};
> +
> +struct bpf_queue {
> + struct bpf_map map;
> + struct list_head head;
> + struct pcpu_freelist freelist;
> + void *nodes;
> + enum queue_type type;
> + raw_spinlock_t lock;
> + atomic_t count;
> + u32 node_size;
> +};
> +
> +struct queue_node {
> + struct pcpu_freelist_node fnode;
> + struct bpf_queue *queue;
> + struct list_head list;
> + struct rcu_head rcu;
> + char element[0] __aligned(8);
> +};
> +
> +static bool queue_map_is_prealloc(struct bpf_queue *queue)
> +{
> + return !(queue->map.map_flags & BPF_F_NO_PREALLOC);
> +}
> +
> +/* Called from syscall */
> +static int queue_map_alloc_check(union bpf_attr *attr)
> +{
> + /* check sanity of attributes */
> + if (attr->max_entries == 0 || attr->key_size != 0 ||
> + attr->value_size == 0 || attr->map_flags & ~QUEUE_CREATE_FLAG_MASK)
> + return -EINVAL;
> +
> + if ((attr->map_flags >> 16) != QUEUE_FIFO &&
> + (attr->map_flags >> 16) != QUEUE_LIFO) {
> + return -EINVAL;
> + }
> +
> + if (attr->value_size > KMALLOC_MAX_SIZE)
> + /* if value_size is bigger, the user space won't be able to
> + * access the elements.
> + */
> + return -E2BIG;
> +
> + return 0;
> +}
> +
> +static int prealloc_init(struct bpf_queue *queue)
> +{
> + u32 node_size = sizeof(struct queue_node) +
> + round_up(queue->map.value_size, 8);
> + u32 num_entries = queue->map.max_entries;
> + int err;
> +
> + queue->nodes = bpf_map_area_alloc(node_size * num_entries,
> + queue->map.numa_node);
> + if (!queue->nodes)
> + return -ENOMEM;
> +
> + err = pcpu_freelist_init(&queue->freelist);
> + if (err)
> + goto free_nodes;
> +
> + pcpu_freelist_populate(&queue->freelist,
> + queue->nodes +
> + offsetof(struct queue_node, fnode),
> + node_size, num_entries);
> +
> + return 0;
> +
> +free_nodes:
> + bpf_map_area_free(queue->nodes);
> + return err;
> +}
> +
> +static void prealloc_destroy(struct bpf_queue *queue)
> +{
> + bpf_map_area_free(queue->nodes);
> + pcpu_freelist_destroy(&queue->freelist);
> +}
> +
> +static struct bpf_map *queue_map_alloc(union bpf_attr *attr)
> +{
> + struct bpf_queue *queue;
> + u64 cost = sizeof(*queue);
> + int ret;
> +
> + queue = kzalloc(sizeof(*queue), GFP_USER);
> + if (!queue)
> + return ERR_PTR(-ENOMEM);
> +
> + bpf_map_init_from_attr(&queue->map, attr);
> +
> + queue->node_size = sizeof(struct queue_node) +
> + round_up(attr->value_size, 8);
> + cost += (u64) attr->max_entries * queue->node_size;
> + if (cost >= U32_MAX - PAGE_SIZE) {
> + ret = -E2BIG;
> + goto free_queue;
> + }
> +
> + queue->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
> +
> + ret = bpf_map_precharge_memlock(queue->map.pages);
> + if (ret)
> + goto free_queue;
> +
> + INIT_LIST_HEAD(&queue->head);
> +
> + raw_spin_lock_init(&queue->lock);
> +
> + queue->type = attr->map_flags >> 16;
> +
> + if (queue_map_is_prealloc(queue))
> + ret = prealloc_init(queue);
> + if (ret)
> + goto free_queue;
> +
> + return &queue->map;
> +
> +free_queue:
> + kfree(queue);
> + return ERR_PTR(ret);
> +}
> +
> +/* Called when map->refcnt goes to zero, either from workqueue or from syscall */
> +static void queue_map_free(struct bpf_map *map)
> +{
> + struct bpf_queue *queue = container_of(map, struct bpf_queue, map);
> + struct queue_node *l;
> +
> + /* at this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
> + * so the programs (can be more than one that used this map) were
> + * disconnected from events. Wait for outstanding critical sections in
> + * these programs to complete
> + */
> + synchronize_rcu();
> +
> + /* some of queue_elem_free_rcu() callbacks for elements of this map may
> + * not have executed. Wait for them.
> + */
> + rcu_barrier();
> + if (!queue_map_is_prealloc(queue))
> + list_for_each_entry_rcu(l, &queue->head, list) {
> + list_del_rcu(&l->list);
> + kfree(l);
> + }
> + else
> + prealloc_destroy(queue);
> + kfree(queue);
> +}
> +
> +static void queue_elem_free_rcu(struct rcu_head *head)
> +{
> + struct queue_node *l = container_of(head, struct queue_node, rcu);
> + struct bpf_queue *queue = l->queue;
> +
> + /* must increment bpf_prog_active to avoid kprobe+bpf triggering while
> + * we're calling kfree, otherwise deadlock is possible if kprobes
> + * are placed somewhere inside of slub
> + */
> + preempt_disable();
> + __this_cpu_inc(bpf_prog_active);
> + if (queue_map_is_prealloc(queue))
> + pcpu_freelist_push(&queue->freelist, &l->fnode);
> + else
> + kfree(l);
> + __this_cpu_dec(bpf_prog_active);
> + preempt_enable();
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static void *queue_map_lookup_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_queue *queue = container_of(map, struct bpf_queue, map);
> + unsigned long flags;
> + struct queue_node *node;
> +
> + raw_spin_lock_irqsave(&queue->lock, flags);
I think a per-cpu flavor would be very useful here as well in this map
type such that we wouldn't need a lock here but only guarantee that
preemption is disabled, and it could be used as temp store for the program
for example.
> + node = list_first_or_null_rcu(&queue->head, struct queue_node, list);
> + if (!node) {
> + raw_spin_unlock_irqrestore(&queue->lock, flags);
> + return NULL;
> + }
> +
> + if (!queue_map_is_prealloc(queue))
> + atomic_dec(&queue->count);
> +
> + list_del_rcu(&node->list);
> + call_rcu(&node->rcu, queue_elem_free_rcu);
> +
> + raw_spin_unlock_irqrestore(&queue->lock, flags);
> +
> + return &node->element;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_map_update_elem(struct bpf_map *map, void *key, void *value,
> + u64 map_flags)
> +{
> + struct bpf_queue *queue = container_of(map, struct bpf_queue, map);
> + unsigned long flags;
> + struct queue_node *new;
Should reject invalid map update flags here.
> + if (!queue_map_is_prealloc(queue)) {
> + if (atomic_inc_return(&queue->count) > queue->map.max_entries) {
> + atomic_dec(&queue->count);
> + return -E2BIG;
> + }
> +
> + new = kmalloc_node(queue->node_size, GFP_ATOMIC | __GFP_NOWARN,
> + queue->map.numa_node);
> + if (!new) {
> + atomic_dec(&queue->count);
> + return -ENOMEM;
> + }
> + } else {
> + struct pcpu_freelist_node *l;
> +
> + l = pcpu_freelist_pop(&queue->freelist);
> + if (!l)
> + return -E2BIG;
> + new = container_of(l, struct queue_node, fnode);
> + }
> +
> + memcpy(new->element, value, queue->map.value_size);
> + new->queue = queue;
> +
> + raw_spin_lock_irqsave(&queue->lock, flags);
> + switch (queue->type) {
> + case QUEUE_FIFO:
> + list_add_tail_rcu(&new->list, &queue->head);
> + break;
> +
> + case QUEUE_LIFO:
> + list_add_rcu(&new->list, &queue->head);
> + break;
> + }
> +
> + raw_spin_unlock_irqrestore(&queue->lock, flags);
> +
> + return 0;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_map_delete_elem(struct bpf_map *map, void *key)
> +{
> + return -EINVAL;
> +}
> +
> +/* Called from syscall */
> +static int queue_map_get_next_key(struct bpf_map *map, void *key,
> + void *next_key)
> +{
> + return -EINVAL;
> +}
> +
> +const struct bpf_map_ops queue_map_ops = {
> + .map_alloc_check = queue_map_alloc_check,
> + .map_alloc = queue_map_alloc,
> + .map_free = queue_map_free,
> + .map_lookup_elem = queue_map_lookup_elem,
> + .map_update_elem = queue_map_update_elem,
> + .map_delete_elem = queue_map_delete_elem,
> + .map_get_next_key = queue_map_get_next_key,
> +};
> +
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index a31a1ba0f8ea..7e9a11d69eef 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -622,11 +622,19 @@ static int map_lookup_elem(union bpf_attr *attr)
> err = -EPERM;
> goto err_put;
> }
> + if (map->map_type != BPF_MAP_TYPE_QUEUE) {
> + key = memdup_user(ukey, map->key_size);
> + if (IS_ERR(key)) {
> + err = PTR_ERR(key);
> + goto err_put;
> + }
> + } else {
> + if (ukey) {
> + err = -EINVAL;
> + goto err_put;
> + }
Given you have a fixed key_size of 0, this could probably be made more
generic and refactored a bit into a helper to check for 0 map->key_size
instead of map type.
> - key = memdup_user(ukey, map->key_size);
> - if (IS_ERR(key)) {
> - err = PTR_ERR(key);
> - goto err_put;
> + key = NULL;
> }
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> @@ -709,10 +717,19 @@ static int map_update_elem(union bpf_attr *attr)
> goto err_put;
> }
>
> - key = memdup_user(ukey, map->key_size);
> - if (IS_ERR(key)) {
> - err = PTR_ERR(key);
> - goto err_put;
> + if (map->map_type != BPF_MAP_TYPE_QUEUE) {
> + key = memdup_user(ukey, map->key_size);
> + if (IS_ERR(key)) {
> + err = PTR_ERR(key);
> + goto err_put;
> + }
> + } else {
> + if (ukey) {
> + err = -EINVAL;
> + goto err_put;
> + }
> +
> + key = NULL;
> }
Ditto, and also further below as well for the other syscall cases.
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> @@ -803,10 +820,19 @@ static int map_delete_elem(union bpf_attr *attr)
> goto err_put;
> }
>
> - key = memdup_user(ukey, map->key_size);
> - if (IS_ERR(key)) {
> - err = PTR_ERR(key);
> - goto err_put;
> + if (map->map_type != BPF_MAP_TYPE_QUEUE) {
> + key = memdup_user(ukey, map->key_size);
> + if (IS_ERR(key)) {
> + err = PTR_ERR(key);
> + goto err_put;
> + }
> + } else {
> + if (ukey) {
> + err = -EINVAL;
> + goto err_put;
> + }
> +
> + key = NULL;
> }
>
> if (bpf_map_is_dev_bound(map)) {
> @@ -855,9 +881,14 @@ static int map_get_next_key(union bpf_attr *attr)
> }
>
> if (ukey) {
> - key = memdup_user(ukey, map->key_size);
> - if (IS_ERR(key)) {
> - err = PTR_ERR(key);
> + if (map->map_type != BPF_MAP_TYPE_QUEUE) {
> + key = memdup_user(ukey, map->key_size);
> + if (IS_ERR(key)) {
> + err = PTR_ERR(key);
> + goto err_put;
> + }
> + } else {
> + err = -EINVAL;
> goto err_put;
> }
> } else {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 25e47c195874..83099a9a21d9 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1976,8 +1976,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> return -EACCES;
> }
>
> - if (arg_type == ARG_PTR_TO_MAP_KEY ||
> - arg_type == ARG_PTR_TO_MAP_VALUE) {
> + if (arg_type == ARG_PTR_TO_MAP_KEY) {
> + expected_type = PTR_TO_STACK;
> + if (!type_is_pkt_pointer(type) && type != PTR_TO_MAP_VALUE &&
> + type != expected_type && type != SCALAR_VALUE)
> + goto err_type;
> + } else if (arg_type == ARG_PTR_TO_MAP_VALUE) {
> expected_type = PTR_TO_STACK;
> if (!type_is_pkt_pointer(type) && type != PTR_TO_MAP_VALUE &&
> type != expected_type)
> @@ -2021,6 +2025,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> /* bpf_map_xxx(map_ptr) call: remember that map_ptr */
> meta->map_ptr = reg->map_ptr;
> } else if (arg_type == ARG_PTR_TO_MAP_KEY) {
> + bool zero_size_allowed = false;
> /* bpf_map_xxx(..., map_ptr, ..., key) call:
> * check that [key, key + map->key_size) are within
> * stack limits and initialized
> @@ -2034,8 +2039,13 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> verbose(env, "invalid map_ptr to access map->key\n");
> return -EACCES;
> }
> +
> + if (meta->map_ptr->map_type == BPF_MAP_TYPE_QUEUE)
Here as well.
> + zero_size_allowed = true;
Also, verifier should rather enforce a const NULL key here than allowing one to
be set (but not as the only 'valid' input option), meaning, I don't think it would
be good to allow a 'zero' sized pointer to stack in this case.
> err = check_helper_mem_access(env, regno,
> - meta->map_ptr->key_size, false,
> + meta->map_ptr->key_size,
> + zero_size_allowed,
> NULL);
> } else if (arg_type == ARG_PTR_TO_MAP_VALUE) {
> /* bpf_map_xxx(..., map_ptr, ..., value) call:
>
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Stefano Brivio @ 2018-08-07 13:39 UTC (permalink / raw)
To: Pravin Shelar
Cc: Matteo Croce, Justin Pettit, Greg Rose, Ben Pfaff, netdev,
ovs dev, Jiri Benc, Aaron
In-Reply-To: <20180807153111.53b0da8d@epycfail>
On Tue, 7 Aug 2018 15:31:11 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> I would instead try to address the concerns that you had about the
> original patch adding fairness in the kernel, rather than trying to
> make the issue appear less severe in ovs-vswitchd.
And, by the way, if we introduce a way to configure the possible
fairness algorithms via netlink, we can also rather easily allow
ovs-vswitchd to disable fairness on kernel side altogether, should it
be needed.
--
Stefano
^ 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