* [PATCH][next] net/mlx5: add missing void argument to function mlx5_devlink_alloc
From: Colin King @ 2019-06-18 15:15 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, David S . Miller, netdev,
linux-rdma
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Function mlx5_devlink_alloc is missing a void argument, add it
to clean up the non-ANSI function declaration.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx5/core/devlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index ed4202e883f0..1533c657220b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -37,7 +37,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
.flash_update = mlx5_devlink_flash_update,
};
-struct devlink *mlx5_devlink_alloc()
+struct devlink *mlx5_devlink_alloc(void)
{
return devlink_alloc(&mlx5_devlink_ops, sizeof(struct mlx5_core_dev));
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net v5 5/6] ipv6: Dump route exceptions if requested
From: David Ahern @ 2019-06-18 15:19 UTC (permalink / raw)
To: Stefano Brivio, David Miller
Cc: Jianlin Shi, Wei Wang, Martin KaFai Lau, Eric Dumazet,
Matti Vaittinen, netdev
In-Reply-To: <364403cca3d7836557f8ffe83c9c48b436be76eb.1560827176.git.sbrivio@redhat.com>
On 6/18/19 7:20 AM, Stefano Brivio wrote:
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 0f60eb3a2873..7375f3b7d310 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4854,33 +4854,94 @@ static bool fib6_info_uses_dev(const struct fib6_info *f6i,
> return false;
> }
>
> -int rt6_dump_route(struct fib6_info *rt, void *p_arg)
> +/* Return -1 if done with node, number of handled routes on partial dump */
> +int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
Changing the return code of rt6_dump_route should be a separate patch.
> {
> struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) p_arg;
> struct fib_dump_filter *filter = &arg->filter;
> + struct rt6_exception_bucket *bucket;
> unsigned int flags = NLM_F_MULTI;
> + struct rt6_exception *rt6_ex;
> struct net *net = arg->net;
> + int i, count = 0;
>
> if (rt == net->ipv6.fib6_null_entry)
> - return 0;
> + return -1;
>
> if ((filter->flags & RTM_F_PREFIX) &&
> !(rt->fib6_flags & RTF_PREFIX_RT)) {
> /* success since this is not a prefix route */
> - return 1;
> + return -1;
> }
> - if (filter->filter_set) {
> - if ((filter->rt_type && rt->fib6_type != filter->rt_type) ||
> - (filter->dev && !fib6_info_uses_dev(rt, filter->dev)) ||
> - (filter->protocol && rt->fib6_protocol != filter->protocol)) {
> - return 1;
> - }
> + if (filter->filter_set &&
> + ((filter->rt_type && rt->fib6_type != filter->rt_type) ||
> + (filter->dev && !fib6_info_uses_dev(rt, filter->dev)) ||
> + (filter->protocol && rt->fib6_protocol != filter->protocol))) {
> + return -1;
> + }
> +
> + if (filter->filter_set ||
> + !filter->dump_routes || !filter->dump_exceptions) {
> flags |= NLM_F_DUMP_FILTERED;
> }
>
> - return rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL, 0,
> - RTM_NEWROUTE, NETLINK_CB(arg->cb->skb).portid,
> - arg->cb->nlh->nlmsg_seq, flags);
> + if (filter->dump_routes) {
> + if (skip) {
> + skip--;
> + } else {
> + if (rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL,
> + 0, RTM_NEWROUTE,
> + NETLINK_CB(arg->cb->skb).portid,
> + arg->cb->nlh->nlmsg_seq, flags)) {
> + return 0;
> + }
> + count++;
> + }
> + }
> +
> + if (!filter->dump_exceptions)
> + return -1;
> +
And the dump of the exception bucket should be a standalone function.
You will see why with net-next (it is per fib6_nh).
> + bucket = rcu_dereference(rt->rt6i_exception_bucket);
> + if (!bucket)
> + return -1;
> +
> + for (i = 0; i < FIB6_EXCEPTION_BUCKET_SIZE; i++) {
> + hlist_for_each_entry(rt6_ex, &bucket->chain, hlist) {
> + if (skip) {
> + skip--;
> + continue;
> + }
> +
> + /* Expiration of entries doesn't bump sernum, insertion
> + * does. Removal is triggered by insertion, so we can
> + * rely on the fact that if entries change between two
> + * partial dumps, this node is scanned again completely,
> + * see rt6_insert_exception() and fib6_dump_table().
> + *
> + * Count expired entries we go through as handled
> + * entries that we'll skip next time, in case of partial
> + * node dump. Otherwise, if entries expire meanwhile,
> + * we'll skip the wrong amount.
> + */
> + if (rt6_check_expired(rt6_ex->rt6i)) {
> + count++;
> + continue;
> + }
> +
> + if (rt6_fill_node(net, arg->skb, rt, &rt6_ex->rt6i->dst,
> + NULL, NULL, 0, RTM_NEWROUTE,
> + NETLINK_CB(arg->cb->skb).portid,
> + arg->cb->nlh->nlmsg_seq, flags)) {
> + return count;
> + }
> +
> + count++;
> + }
> + bucket++;
> + }
> +
> + return -1;
> }
>
> static int inet6_rtm_valid_getroute_req(struct sk_buff *skb,
>
^ permalink raw reply
* Re: [PATCH net-next v1 08/11] xdp: tracking page_pool resources and safe removal
From: Jesper Dangaard Brouer @ 2019-06-18 15:19 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Tariq Toukan, netdev@vger.kernel.org, Ilias Apalodimas,
Toke Høiland-Jørgensen, toshiaki.makita1@gmail.com,
grygorii.strashko@ti.com, mcroce@redhat.com, brouer
In-Reply-To: <20190618125431.GA5307@khorivan>
On Tue, 18 Jun 2019 15:54:33 +0300 Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> On Sun, Jun 16, 2019 at 10:56:25AM +0000, Tariq Toukan wrote:
> >
> >On 6/15/2019 12:33 PM, Ivan Khoronzhuk wrote:
> >> On Thu, Jun 13, 2019 at 08:28:42PM +0200, Jesper Dangaard Brouer wrote:
[...]
> >>
> >> What would you recommend to do for the following situation:
> >>
> >> Same receive queue is shared between 2 network devices. The receive ring is
> >> filled by pages from page_pool, but you don't know the actual port (ndev)
> >> filling this ring, because a device is recognized only after packet is
> >> received.
> >>
> >> The API is so that xdp rxq is bind to network device, each frame has
> >> reference
> >> on it, so rxq ndev must be static. That means each netdev has it's own rxq
> >> instance even no need in it. Thus, after your changes, page must be
> >> returned to
> >> the pool it was taken from, or released from old pool and recycled in
> >> new one
> >> somehow.
> >>
> >> And that is inconvenience at least. It's hard to move pages between
> >> pools w/o performance penalty. No way to use common pool either,
> >> as unreg_rxq now drops the pool and 2 rxqa can't reference same
> >> pool.
> >
> >Within the single netdev, separate page_pool instances are anyway
> >created for different RX rings, working under different NAPI's.
>
> The circumstances are so that same RX ring is shared between 2
> netdevs... and netdev can be known only after descriptor/packet is
> received. Thus, while filling RX ring, there is no actual device,
> but when packet is received it has to be recycled to appropriate
> net device pool. Before this change there were no difference from
> which pool the page was allocated to fill RX ring, as there were no
> owner. After this change there is owner - netdev page pool.
It not really a dependency added in this patchset. A page_pool is
strictly bound to a single RX-queue, for performance, as this allow us
a NAPI fast-path return used for early drop (XDP_DROP).
I can see that the API xdp_rxq_info_reg_mem_model() make it possible to
call it on different xdp_rxq_info structs with the same page_pool
pointer. But it was never intended to be used like that, and I
consider it an API usage violation. I originally wanted to add the
allocator pointer to xdp_rxq_info_reg() call, but the API was extended
in different versions, so I didn't want to break users. I've actually
tried hard to catch when drivers use the API wrong, via WARN(), but I
guess you found a loop hole.
Besides, we already have a dependency from the RX-queue to the netdev
in the xdp_rxq_info structure. E.g. the xdp_rxq_info->dev is sort of
central, and dereferenced by BPF-code to read xdp_md->ingress_ifindex,
and also used by cpumap when creating SKBs.
> For cpsw the dma unmap is common for both netdevs and no difference
> who is freeing the page, but there is difference which pool it's
> freed to.
>
> So that, while filling RX ring the page is taken from page pool of
> ndev1, but packet is received for ndev2, it has to be later
> returned/recycled to page pool of ndev1, but when xdp buffer is
> handed over to xdp prog the xdp_rxq_info has reference on ndev2 ...
>
> And no way to predict the final ndev before packet is received, so no
> way to choose appropriate page pool as now it becomes page owner.
>
> So, while RX ring filling, the page/dma recycling is needed but should
> be some way to identify page owner only after receiving packet.
>
> Roughly speaking, something like:
>
> pool->pages_state_hold_cnt++;
>
> outside of page allocation API, after packet is received.
Don't EVER manipulate the internal state outside of page allocation
API. That kills the purpose of defining any API.
> and free of the counter while allocation (w/o owing the page).
You use-case of two netdev's sharing the same RX-queue sounds dubious,
and very hardware specific. I'm not sure why we want to bend the APIs
to support this?
If we had to allow page_pool to be registered twice, via
xdp_rxq_info_reg_mem_model() then I guess we could extend page_pool
with a usage/users reference count, and then only really free the
page_pool when refcnt reach zero. But it just seems and looks wrong
(in the code) as the hole trick to get performance is to only have one
user.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-18 15:20 UTC (permalink / raw)
To: Johannes Berg, Arnd Bergmann, Dan Williams
Cc: Subash Abhinov Kasiviswanathan, abhishek.esse, Ben Chan,
Bjorn Andersson, cpratapa, David Miller, DTML, Eric Caruso,
evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <e6ba8a9063e63506c0b88a70418d74ca4efe85cd.camel@sipsolutions.net>
On 6/17/19 7:25 AM, Johannes Berg wrote:
> On Mon, 2019-06-17 at 13:42 +0200, Johannes Berg wrote:
>
>> But anyway, as I alluded to above, I had something like this in mind:
>
> I forgot to state this here, but this was *heavily* influenced by
> discussions with Dan - many thanks to him.
Thanks for getting even more concrete with this. Code is the
most concise way of describing things, once the general ideas
seem to be coming together.
I'm not going to comment on the specific code bits, but I have
some more general questions and comments on the design. Some
of these are simply due to my lack of knowledge of how WWAN/modem
interactions normally work.
First, a few terms (correct or improve as you like):
- WWAN device is a hardware device (like IPA) that presents a
connection between AP and modem, and presents an interface
that allows the use of that connection to be managed.
- WWAN netdevice represents a Linux network interface, with its
operations and queues, etc., but implements a standardized
set of WWAN-specific operations. It represents a logical
' channel whose data is multiplexed over the WWAN device.
- WWAN channel is a user space abstraction that corresponds
with a WWAN netdevice (but I'm not clear on all the ways
they differ or interact).
- The WWAN core is kernel code that presents abstractions
for WWAN devices and netdevices, so they can be managed
in a generic way. It is for configuration and communication
and is not at all involved in the data path.
You're saying that the WWAN driver space calls wwan_add()
to register itself as a new WWAN device.
You're also saying that a WWAN device "attaches" a WWAN
netdevice, which is basically notifying the WWAN core
that the new netdev/channel is available for use.
- I trust that a "tentative" attachement is necessary. But
I'm not sure what makes it transition into becoming a
"real" one, or how that event gets communicated.
Some questions:
- What causes a new channel to be created? Is it initiated
by the WWAN device driver? Does the modem request that
it get created? User space? Both?
- What causes a created channel to be removed?
- You distinguish between attaching a netdevice and (what
I'll call) activating it. What causes activation?
- How are the attributes of a WWAN device or channel set,
or communicated?
- Are there any attributes that are only optionally supported,
and if so, how are the supported ones communicated?
- Which WWAN channel attributes must be set *before* the
channel is activated, and can't be changed? Are there any
that can be changed dynamically?
And while the whole point of this is to make things generic,
it might be nice to have a way to implement a new feature
before it can be "standardized".
Thanks.
-Alex
PS I don't want to exclude anybody but we could probably start
a different mail chain on this topic...
>> driver_dev
>> struct device *dev (USB, PCI, ...)
>> net_device NA
>> net_device NB
>> tty TA
>> ...
>>
. . .
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic
From: Jon Hunter @ 2019-06-18 15:20 UTC (permalink / raw)
To: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Russell King, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-tegra
In-Reply-To: <b66c7578-172f-4443-f4c3-411525e28738@nvidia.com>
On 18/06/2019 11:18, Jon Hunter wrote:
>
> On 18/06/2019 10:46, Jose Abreu wrote:
>> From: Jon Hunter <jonathanh@nvidia.com>
>>
>>> I am not certain but I don't believe so. We are using a static IP address
>>> and mounting the root file-system via NFS when we see this ...
>>
>> Can you please add a call to napi_synchronize() before every
>> napi_disable() calls, like this:
>>
>> if (queue < rx_queues_cnt) {
>> napi_synchronize(&ch->rx_napi);
>> napi_disable(&ch->rx_napi);
>> }
>>
>> if (queue < tx_queues_cnt) {
>> napi_synchronize(&ch->tx_napi);
>> napi_disable(&ch->tx_napi);
>> }
>>
>> [ I can send you a patch if you prefer ]
>
> Yes I can try this and for completeness you mean ...
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4ca46289a742..d4a12cb64d8e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
> for (queue = 0; queue < maxq; queue++) {
> struct stmmac_channel *ch = &priv->channel[queue];
>
> - if (queue < rx_queues_cnt)
> + if (queue < rx_queues_cnt) {
> + napi_synchronize(&ch->rx_napi);
> napi_disable(&ch->rx_napi);
> - if (queue < tx_queues_cnt)
> + }
> +
> + if (queue < tx_queues_cnt) {
> + napi_synchronize(&ch->tx_napi);
> napi_disable(&ch->tx_napi);
> + }
> }
> }
So good news and bad news ...
The good news is that the above change does fix the initial crash
I am seeing. However, even with this change applied on top of
-next, it is still dying somewhere else and so there appears to
be a second issue.
On a successful boot I see ...
[ 6.150419] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR clock
[ 6.156441] dwc-eth-dwmac 2490000.ethernet: no reset control found
[ 6.175866] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10, Synopsys ID: 0x41
[ 6.182912] dwc-eth-dwmac 2490000.ethernet: DWMAC4/5
[ 6.187961] dwc-eth-dwmac 2490000.ethernet: DMA HW capability register supported
[ 6.195351] dwc-eth-dwmac 2490000.ethernet: RX Checksum Offload Engine supported
[ 6.202735] dwc-eth-dwmac 2490000.ethernet: TX Checksum insertion supported
[ 6.209685] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan supported
[ 6.216041] dwc-eth-dwmac 2490000.ethernet: TSO supported
[ 6.221433] dwc-eth-dwmac 2490000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[ 6.229342] dwc-eth-dwmac 2490000.ethernet: device MAC address 9a:9b:49:6f:a5:ee
[ 6.236727] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled
[ 6.242689] libphy: stmmac: probed
On the latest -next with the patch applied I see ...
[ 6.043529] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR clock
[ 6.049546] dwc-eth-dwmac 2490000.ethernet: no reset control found
[ 6.068895] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10, Synopsys ID: 0x41
[ 6.075941] dwc-eth-dwmac 2490000.ethernet: DWMAC4/5
[ 6.080989] dwc-eth-dwmac 2490000.ethernet: DMA HW capability register supported
[ 6.088373] dwc-eth-dwmac 2490000.ethernet: RX Checksum Offload Engine supported
[ 6.095756] dwc-eth-dwmac 2490000.ethernet: TX Checksum insertion supported
[ 6.102708] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan supported
[ 6.109074] dwc-eth-dwmac 2490000.ethernet: TSO supported
[ 6.114465] dwc-eth-dwmac 2490000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[ 6.122373] dwc-eth-dwmac 2490000.ethernet: device MAC address ee:3a:9a:b0:7e:34
[ 6.129756] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled
And it dies here. No more output is seen. I will try to figure
out which commit is causing this issue.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH net-next v2 03/16] ipv6: Extend notifier info for multipath routes
From: David Ahern @ 2019-06-18 15:26 UTC (permalink / raw)
To: Ido Schimmel, netdev; +Cc: davem, jiri, alexpe, mlxsw, Ido Schimmel
In-Reply-To: <20190618151258.23023-4-idosch@idosch.org>
On 6/18/19 9:12 AM, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
>
> Extend the IPv6 FIB notifier info with number of sibling routes being
> notified.
>
> This will later allow listeners to process one notification for a
> multipath routes instead of N, where N is the number of nexthops.
>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/net/ip6_fib.h | 7 +++++++
> net/ipv6/ip6_fib.c | 17 +++++++++++++++++
> 2 files changed, 24 insertions(+)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v2 15/16] ipv6: Stop sending in-kernel notifications for each nexthop
From: David Ahern @ 2019-06-18 15:28 UTC (permalink / raw)
To: Ido Schimmel, netdev; +Cc: davem, jiri, alexpe, mlxsw, Ido Schimmel
In-Reply-To: <20190618151258.23023-16-idosch@idosch.org>
On 6/18/19 9:12 AM, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
>
> Both listeners - mlxsw and netdevsim - of IPv6 FIB notifications are now
> ready to handle IPv6 multipath notifications.
>
> Therefore, stop ignoring such notifications in both drivers and stop
> sending notification for each added / deleted nexthop.
>
> v2:
> * Remove 'multipath_rt' from 'struct fib6_entry_notifier_info'
>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> .../ethernet/mellanox/mlxsw/spectrum_router.c | 2 --
> drivers/net/netdevsim/fib.c | 7 -----
> include/net/ip6_fib.h | 1 -
> net/ipv6/ip6_fib.c | 29 +++++++++++--------
> 4 files changed, 17 insertions(+), 22 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH RESEND nf-next] netfilter: add support for matching IPv4 options
From: Pablo Neira Ayuso @ 2019-06-18 15:31 UTC (permalink / raw)
To: Stephen Suryaputra; +Cc: netfilter-devel, netdev
In-Reply-To: <20190611120912.3825-1-ssuryaextr@gmail.com>
On Tue, Jun 11, 2019 at 08:09:12AM -0400, Stephen Suryaputra wrote:
[...]
> diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
> index a940c9fd9045..4155a32fade7 100644
> --- a/net/netfilter/nft_exthdr.c
> +++ b/net/netfilter/nft_exthdr.c
> @@ -62,6 +62,125 @@ static void nft_exthdr_ipv6_eval(const struct nft_expr *expr,
> regs->verdict.code = NFT_BREAK;
> }
>
> +/* find the offset to specified option or the header beyond the options
> + * if target < 0.
> + *
> + * If target header is found, its offset is set in *offset and return option
> + * number. Otherwise, return negative error.
> + *
> + * If the first fragment doesn't contain the End of Options it is considered
> + * invalid.
> + */
> +static int ipv4_find_option(struct net *net, struct sk_buff *skb,
> + unsigned int *offset, int target,
> + unsigned short *fragoff, int *flags)
flags is never used, please remove it.
> +{
> + unsigned char optbuf[sizeof(struct ip_options) + 41];
In other parts of the kernel this is + 40:
net/ipv4/cipso_ipv4.c: unsigned char optbuf[sizeof(struct ip_options) + 40];
here it is + 41.
> + struct ip_options *opt = (struct ip_options *)optbuf;
> + struct iphdr *iph, _iph;
> + unsigned int start;
> + bool found = false;
> + __be32 info;
> + int optlen;
> +
> + if (fragoff)
> + *fragoff = 0;
fragoff is set and never used. Please, remove this parameter.
> + iph = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> + if (!iph || iph->version != 4)
> + return -EBADMSG;
> + start = sizeof(struct iphdr);
> +
> + optlen = iph->ihl * 4 - (int)sizeof(struct iphdr);
> + if (optlen <= 0)
> + return -ENOENT;
> +
> + memset(opt, 0, sizeof(struct ip_options));
> + /* Copy the options since __ip_options_compile() modifies
> + * the options. Get one byte beyond the option for target < 0
How does this "one byte beyond the option" trick works?
> + */
> + if (skb_copy_bits(skb, start, opt->__data, optlen + 1))
> + return -EBADMSG;
> + opt->optlen = optlen;
> +
> + if (__ip_options_compile(net, opt, NULL, &info))
> + return -EBADMSG;
> +
> + switch (target) {
> + case IPOPT_SSRR:
> + case IPOPT_LSRR:
> + if (!opt->srr)
> + break;
> + found = target == IPOPT_SSRR ? opt->is_strictroute :
> + !opt->is_strictroute;
> + if (found)
> + *offset = opt->srr + start;
> + break;
> + case IPOPT_RR:
> + if (opt->rr)
> + break;
> + *offset = opt->rr + start;
> + found = true;
> + break;
> + case IPOPT_RA:
> + if (opt->router_alert)
> + break;
> + *offset = opt->router_alert + start;
> + found = true;
> + break;
> + default:
> + /* Either not supported or not a specific search, treated as
> + * found
> + */
> + found = true;
> + if (target >= 0) {
> + target = -EOPNOTSUPP;
> + break;
> + }
> + if (opt->end) {
> + *offset = opt->end + start;
> + target = IPOPT_END;
May I ask, what's the purpose of IPOPT_END? :-)
> + } else {
> + /* Point to beyond the options. */
> + *offset = optlen + start;
> + target = opt->__data[optlen];
> + }
> + }
> + if (!found)
> + target = -ENOENT;
> + return target;
nitpick: Probably replace code above.
return found ? target : -ENOENT;
Apart from the above, this looks good to me.
Thanks!
^ permalink raw reply
* Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG
From: Pablo Neira Ayuso @ 2019-06-18 15:33 UTC (permalink / raw)
To: wenxu; +Cc: Florian Westphal, netfilter-devel, netdev
In-Reply-To: <591caf69-ba08-33b5-5330-8230779cc903@ucloud.cn>
On Tue, Jun 18, 2019 at 10:27:12PM +0800, wenxu wrote:
>
> 在 2019/6/18 17:37, Florian Westphal 写道:
> > wenxu <wenxu@ucloud.cn> wrote:
> >> On 6/18/2019 6:42 AM, Florian Westphal wrote:
> >>> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >>>>> Subject: Change bridge l3 dependency to meta protocol
> >>>>>
> >>>>> This examines skb->protocol instead of ethernet header type, which
> >>>>> might be different when vlan is involved.
> >>>>>
> >>>>> + if (ctx->pctx.family == NFPROTO_BRIDGE && desc == &proto_eth) {
> >>>>> + if (expr->payload.desc == &proto_ip ||
> >>>>> + expr->payload.desc == &proto_ip6)
> >>>>> + desc = &proto_metaeth;
> >>>>> + }i
> >>>> Is this sufficient to restrict the matching? Is this still buggy from
> >>>> ingress?
> >>> This is what netdev family uses as well (skb->protocol i mean).
> >>> I'm not sure it will work for output however (haven't checked).
> >>>
> >>>> I wonder if an explicit NFT_PAYLOAD_CHECK_VLAN flag would be useful in
> >>>> the kernel, if so we could rename NFTA_PAYLOAD_CSUM_FLAGS to
> >>>> NFTA_PAYLOAD_FLAGS and place it there. Just an idea.
> >>> Another unresolved issue is presence of multiple vlan tags, so we might
> >>> have to add yet another meta key to retrieve the l3 protocol in use
> >> Maybe add a l3proto meta key can handle the multiple vlan tags case with the l3proto dependency. It
> >> should travese all the vlan tags and find the real l3 proto.
> > Yes, something like this.
> >
> > We also need to audit netdev and bridge expressions (reject is known broken)
> > to handle vlans properly.
> >
> > Still, switching nft to prefer skb->protocol instead of eth_hdr->type
> > for dependencies would be good as this doesn't need kernel changes and solves
> > the immediate problem of 'ip ...' not matching in case of vlan.
> >
> > If you have time, could you check if using skb->protocol works for nft
> > bridge in output, i.e. does 'nft ip protocol icmp' match when its used
> > from bridge output path with meta protocol dependency with and without
> > vlan in use?
>
> I just check the kernel codes and test with the output chain, the
> meta protocol dependency can also work in the outpu chain.
OK.
Florian, would you submit a patch, including a test for this?
Thanks!
^ permalink raw reply
* Re: [iproute2 net-next PATCH] ip: add a new parameter -Numeric
From: David Ahern @ 2019-06-18 15:41 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Stephen Hemminger, Phil Sutter
In-Reply-To: <20190612092115.30043-1-liuhangbin@gmail.com>
On 6/12/19 3:21 AM, Hangbin Liu wrote:
> Add a new parameter '-Numeric' to show the number of protocol, scope,
> dsfield, etc directly instead of converting it to human readable name.
> Do the same on tc and ss.
>
> This patch is based on David Ahern's previous patch.
>
> Suggested-by: Phil Sutter <phil@nwl.cc>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> include/utils.h | 1 +
> ip/ip.c | 6 +++++-
> ip/rtm_map.c | 6 ++++++
> lib/inet_proto.c | 2 +-
> lib/ll_proto.c | 2 +-
> lib/ll_types.c | 3 ++-
> lib/rt_names.c | 18 ++++++++++--------
> man/man8/ip.8 | 6 ++++++
> man/man8/tc.8 | 6 ++++++
> misc/ss.c | 15 ++++++---------
> tc/tc.c | 5 ++++-
> 11 files changed, 48 insertions(+), 22 deletions(-)
>
applied to iproute2-next.Thanks
^ permalink raw reply
* Re: [PATCH iproute2 v2 0/3] refactor the cmd_exec()
From: Matteo Croce @ 2019-06-18 15:41 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, David Ahern, Andrea Claudi
In-Reply-To: <20190618144935.31405-1-mcroce@redhat.com>
On Tue, Jun 18, 2019 at 4:49 PM Matteo Croce <mcroce@redhat.com> wrote:
>
> Refactor the netns and ipvrf code so less steps are needed to exec commands
> in a netns or a VRF context.
> Also remove some code which became dead. bloat-o-meter shows a tiny saving.
>
> Matteo Croce (3):
> netns: switch netns in the child when executing commands
> ip vrf: use hook to change VRF in the child
> netns: make netns_{save,restore} static
>
> include/namespace.h | 2 --
> include/utils.h | 6 ++---
> ip/ip.c | 1 -
> ip/ipnetns.c | 61 ++++++++++++++++++++++++++++++++++-----------
> ip/ipvrf.c | 12 ++++++---
> lib/exec.c | 7 +++++-
> lib/namespace.c | 31 -----------------------
> lib/utils.c | 27 --------------------
> 8 files changed, 63 insertions(+), 84 deletions(-)
>
> --
> 2.21.0
>
Hi all,
this should really be the v3, I did an off-by-one.
Sorry,
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH 00/15] kbuild: refactor headers_install and support compile-test of UAPI headers
From: Masahiro Yamada @ 2019-06-18 15:46 UTC (permalink / raw)
To: Linux Kbuild mailing list
Cc: Song Liu, open list:DOCUMENTATION, Benjamin Herrenschmidt,
Palmer Dabbelt, Heiko Carstens, Alexei Starovoitov, David Howells,
Paul Mackerras, linux-riscv, Vincent Chen, Sam Ravnborg,
linux-s390, Vasily Gorbik, Daniel Borkmann, Jonathan Corbet,
Michael Ellerman, Helge Deller, Christian Borntraeger,
Yonghong Song, arcml, Albert Ou, Arnd Bergmann, Jani Nikula,
Greentime Hu, James E.J. Bottomley, Michal Marek, linux-parisc,
Vineet Gupta, Randy Dunlap, Linux Kernel Mailing List, Networking,
bpf, linuxppc-dev, Martin KaFai Lau
In-Reply-To: <20190604101409.2078-1-yamada.masahiro@socionext.com>
On Tue, Jun 4, 2019 at 7:15 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
>
> Multiple people have suggested to compile-test UAPI headers.
>
> Currently, Kbuild provides simple sanity checks by headers_check
> but they are not enough to catch bugs.
>
> The most recent patch I know is David Howells' work:
> https://patchwork.kernel.org/patch/10590203/
>
> I agree that we need better tests for UAPI headers,
> but I want to integrate it in a clean way.
>
> The idea that has been in my mind is to compile each header
> to make sure the selfcontainedness.
>
> Recently, Jani Nikula proposed a new syntax 'header-test-y'.
> https://patchwork.kernel.org/patch/10947005/
>
> So, I implemented UAPI compile-testing on top of that.
>
> When adding a new feature, cleaning the code first is a
> good practice.
>
> [1] Remove headers_install_all
>
> This target installs UAPI headers of all architectures
> in a single tree.
> It does not make sense to compile test of headers from
> multiple arches at the same time. Hence, removed.
>
> [2] Split header installation into 'make headers' and 'make headers_install'
>
> To compile-test UAPI headers, we need a work-directory somewhere
> to save objects and .*.cmd files.
>
> usr/include/ will be the work-directory.
>
> Since we cannot pollute the final destination of headers_install,
>
> I split the header installation into two stages.
>
> 'make headers' will build up
> the ready-to-install headers in usr/include,
> which will be also used as a work-directory for the compile-test.
>
> 'make headers_install' will copy headers
> from usr/include to $(INSTALL_HDR_PATH)/include.
>
> [3] Support compile-test of UAPI headers
>
> This is implemented in usr/include/Makefile
>
>
> Jani Nikula (1):
> kbuild: add support for ensuring headers are self-contained
>
> Masahiro Yamada (14):
> kbuild: remove headers_{install,check}_all
> kbuild: remove stale dependency between Documentation/ and
> headers_install
> kbuild: make gdb_script depend on prepare0 instead of prepare
> kbuild: fix Kconfig prompt of CONFIG_HEADERS_CHECK
> kbuild: add CONFIG_HEADERS_INSTALL and loosen the dependency of
> samples
> kbuild: remove build_unifdef target in scripts/Makefile
> kbuild: build all prerequisite of headers_install simultaneously
> kbuild: add 'headers' target to build up ready-to-install uapi headers
> kbuild: re-implement Makefile.headersinst without directory descending
> kbuild: move hdr-inst shorthand to top Makefile
> kbuild: simplify scripts/headers_install.sh
> kbuild: deb-pkg: do not run headers_check
> fixup: kbuild: add support for ensuring headers are self-contained
> kbuild: compile test UAPI headers to ensure they are self-contained
Series, applied to linux-kbuild.
> Documentation/kbuild/headers_install.txt | 7 --
> Documentation/kbuild/makefiles.txt | 13 ++-
> Makefile | 56 +++++-----
> arch/arc/configs/tb10x_defconfig | 1 +
> arch/nds32/configs/defconfig | 1 +
> arch/parisc/configs/a500_defconfig | 1 +
> arch/parisc/configs/b180_defconfig | 1 +
> arch/parisc/configs/c3000_defconfig | 1 +
> arch/parisc/configs/default_defconfig | 1 +
> arch/powerpc/configs/ppc6xx_defconfig | 1 +
> arch/s390/configs/debug_defconfig | 1 +
> include/uapi/{linux => }/Kbuild | 6 +-
> init/Kconfig | 20 ++++
> lib/Kconfig.debug | 25 +++--
> samples/Kconfig | 14 ++-
> samples/Makefile | 4 +-
> scripts/Kbuild.include | 6 --
> scripts/Makefile | 5 -
> scripts/Makefile.build | 9 ++
> scripts/Makefile.headersinst | 132 ++++++++++-------------
> scripts/Makefile.lib | 3 +
> scripts/cc-system-headers.sh | 8 ++
> scripts/headers.sh | 29 -----
> scripts/headers_install.sh | 48 ++++-----
> scripts/package/builddeb | 2 +-
> usr/.gitignore | 1 -
> usr/Makefile | 2 +
> usr/include/.gitignore | 3 +
> usr/include/Makefile | 132 +++++++++++++++++++++++
> 29 files changed, 329 insertions(+), 204 deletions(-)
> rename include/uapi/{linux => }/Kbuild (77%)
> create mode 100755 scripts/cc-system-headers.sh
> delete mode 100755 scripts/headers.sh
> create mode 100644 usr/include/.gitignore
> create mode 100644 usr/include/Makefile
>
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum_ptp: Fix compilation on 32-bit ARM
From: David Miller @ 2019-06-18 15:56 UTC (permalink / raw)
To: shalomt; +Cc: netdev, jiri, idosch, mlxsw, natechancellor
In-Reply-To: <20190618124521.22612-1-shalomt@mellanox.com>
From: Shalom Toledo <shalomt@mellanox.com>
Date: Tue, 18 Jun 2019 12:45:35 +0000
> Compilation on 32-bit ARM fails after commit 992aa864dca0 ("mlxsw:
> spectrum_ptp: Add implementation for physical hardware clock operations")
> because of 64-bit division:
>
> arm-linux-gnueabi-ld:
> drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.o: in function
> `mlxsw_sp1_ptp_phc_settime': spectrum_ptp.c:(.text+0x39c): undefined
> reference to `__aeabi_uldivmod'
>
> Fix by using div_u64().
>
> Fixes: 992aa864dca0 ("mlxsw: spectrum_ptp: Add implementation for physical hardware clock operations")
> Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v3] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Pablo Neira Ayuso @ 2019-06-18 15:57 UTC (permalink / raw)
To: linmiaohe
Cc: kadlec, fw, davem, kuznet, yoshfuji, netfilter-devel, coreteam,
netdev, linux-kernel, dsahern, Mingfangsen
In-Reply-To: <212e4feb-39de-2627-9948-bbb117ff4d4e@huawei.com>
On Thu, Apr 25, 2019 at 09:43:53PM +0800, linmiaohe wrote:
> From: Miaohe Lin <linmiaohe@huawei.com>
>
> When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
> ipv4/ipv6 packets will be dropped because in device is
> vrf but out device is an enslaved device. So failed with
> the check of the rpfilter.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> net/ipv4/netfilter/ipt_rpfilter.c | 1 +
> net/ipv6/netfilter/ip6t_rpfilter.c | 10 +++++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
> index 0b10d8812828..6e07cd0ecbec 100644
> --- a/net/ipv4/netfilter/ipt_rpfilter.c
> +++ b/net/ipv4/netfilter/ipt_rpfilter.c
> @@ -81,6 +81,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
> flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
> flow.flowi4_tos = RT_TOS(iph->tos);
> flow.flowi4_scope = RT_SCOPE_UNIVERSE;
> + flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
>
> return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par), info->flags) ^ invert;
> }
> diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
> index c3c6b09acdc4..a28c81322148 100644
> --- a/net/ipv6/netfilter/ip6t_rpfilter.c
> +++ b/net/ipv6/netfilter/ip6t_rpfilter.c
> @@ -58,7 +58,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> if (rpfilter_addr_linklocal(&iph->saddr)) {
> lookup_flags |= RT6_LOOKUP_F_IFACE;
> fl6.flowi6_oif = dev->ifindex;
> - } else if ((flags & XT_RPFILTER_LOOSE) == 0)
> + } else if (((flags & XT_RPFILTER_LOOSE) == 0) ||
> + (netif_is_l3_master(dev)) ||
> + (netif_is_l3_slave(dev)))
> fl6.flowi6_oif = dev->ifindex;
>
> rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags);
> @@ -73,6 +75,12 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
> goto out;
> }
>
> + if (netif_is_l3_master(dev)) {
> + dev = dev_get_by_index_rcu(dev_net(dev), IP6CB(skb)->iif);
> + if (!dev)
> + goto out;
> + }
So, for the l3 device cases this makes:
#1 ip6_route_lookup() to fetch the route, using the device in xt_in()
(the _LOOSE flag is ignored for the l3 device case).
#2 If this is a l3dev master, then you make a global lookup for the
device using IP6CB(skb)->iif.
#3 You check if route matches with the device, using the new device
from the lookup:
if (rt->rt6i_idev->dev == dev ...
If there is no other way to fix this, OK, that's fair enough.
Still this fix looks a bit tricky to me.
And this assymmetric between the IPv4 and IPv6 codebase looks rare.
Probably someone can explain me this in more detail? I'd appreciate.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/3] net/sched: Introduce action ct
From: Cong Wang @ 2019-06-18 16:03 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Toke Høiland-Jørgensen, Paul Blakey, Jiri Pirko,
Roi Dayan, Yossi Kuperman, Oz Shlomo, netdev@vger.kernel.org,
David Miller, Aaron Conole, Zhike Wang, Rony Efraim,
nst-kernel@redhat.com, John Hurley, Simon Horman, Justin Pettit,
Kevin Darbyshire-Bryant
In-Reply-To: <20190614192403.GK3436@localhost.localdomain>
On Fri, Jun 14, 2019 at 12:24 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Fri, Jun 14, 2019 at 11:07:37AM -0700, Cong Wang wrote:
> > On Tue, Jun 11, 2019 at 9:44 AM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > > I had suggested to let act_ct handle the above as well, as there is a
> > > big chunk of code on both that is pretty similar. There is quite some
> > > boilerplate for interfacing with conntrack which is duplicated.
> >
> > Why do you want to mix retrieving conntrack info with executing
> > conntrack?
>
> To save on the heavy boilerplate for interfacing with conntrack.
>
> >
> > They are totally different things to me, act_ctinfo merely retrieves
> > information from conntrack, while this one, act_ct, is supposed to
> > move packets to conntrack.
>
> Seems we have a different understanding for "move packets to
> conntrack": conntrack will not consume the packets after this.
> But after act_ct is executed, if not with the clear flag, skb will now
> have the skb->_nfct entry available, on which flower then will be able
> to match. So in essence, it is also fetching information from
> conntrack.
Interesting. Is it because cls_flower uses conntrack for flow dissection?
What's the reason behind?
Again, I am still not convinced to do L3 operations in L2, skb->_nfct
belongs to conntrack which is L3, no matter the packet is consumed
or not.
Thanks.
^ permalink raw reply
* [PATCH] net: mvpp2: cls: Add pmap to fs dump
From: Nathan Huckleberry @ 2019-06-18 16:09 UTC (permalink / raw)
To: davem, maxime.chevallier
Cc: netdev, linux-kernel, Nathan Huckleberry, clang-built-linux
In-Reply-To: <20190618083900.78eb88bd@bootlin.com>
There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
Added a usage consistent with other fops to dump pmap
to userspace.
Cc: clang-built-linux@googlegroups.com
Link: https://github.com/ClangBuiltLinux/linux/issues/529
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 0ee39ea47b6b..55947bc63cfd 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -566,6 +566,9 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
debugfs_create_file("hits", 0444, prs_entry_dir, entry,
&mvpp2_dbgfs_prs_hits_fops);
+ ddebugfs_create_file("pmap", 0444, prs_entry_dir, entry,
+ &mvpp2_dbgfs_prs_pmap_fops);
+
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: 4.19: udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
From: Greg KH @ 2019-06-18 16:10 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Naresh Kamboju, David S. Miller, Netdev, open list,
open list:KERNEL SELFTEST FRAMEWORK, Fred Klassen
In-Reply-To: <CA+FuTSfBFqRViKfG5crEv8xLMgAkp3cZ+yeuELK5TVv61xT=Yw@mail.gmail.com>
On Tue, Jun 18, 2019 at 08:31:16AM -0400, Willem de Bruijn wrote:
> On Tue, Jun 18, 2019 at 7:27 AM Naresh Kamboju
> <naresh.kamboju@linaro.org> wrote:
> >
> > selftests: net: udpgso_bench.sh failed on 4.19, 4.14, 4.9 and 4.4 branches.
> > PASS on stable branch 5.1, mainline and next.
> > This failure is started happening on 4.19 and older kernel branches after
> > kselftest upgrade to version 5.1
>
> Does version 5.1 here mean running tests from Linux 5.1, against older kernels?
>
> > Is there any possibilities to backport ?
> >
> > Error:
> > udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
>
> MSG_ZEROCOPY for UDP was added in commit b5947e5d1e71 ("udp:
> msg_zerocopy") in Linux 5.0.
>
> The selftest was expanded with this feature in commit db63e489c7aa
> ("selftests: extend zerocopy tests to udp"), also in Linux 5.0.
>
> Those tests are not expected to pass on older kernels.
Any way to degrade gracefully if the feature is not present at all in
the kernel under test? People run the latest version of kselftests on
older kernels all the time.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net v5 0/6] Fix listing (IPv4, IPv6) and flushing (IPv6) of cached route exceptions
From: David Miller @ 2019-06-18 16:25 UTC (permalink / raw)
To: dsahern; +Cc: sbrivio, jishi, weiwan, kafai, edumazet, matti.vaittinen, netdev
In-Reply-To: <a88182c7-10ee-0505-3b5b-bec852e24e97@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Tue, 18 Jun 2019 08:51:01 -0600
> Changing the dump code has been notoriously tricky to get right in one
> go, no matter how much testing you have done. Given that I think this
> should go to net-next first and once it proves ok there we can look at a
> backport to stable trees.
I agree, this is probably the wisest way forward with these changes.
^ permalink raw reply
* Re: [PATCH bpf-next v6 1/9] bpf: implement getsockopt and setsockopt hooks
From: Alexei Starovoitov @ 2019-06-18 16:31 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, bpf, davem, ast, daniel, Martin Lau
In-Reply-To: <20190617180109.34950-2-sdf@google.com>
On Mon, Jun 17, 2019 at 11:01:01AM -0700, Stanislav Fomichev wrote:
> Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
> BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
>
> BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
> BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
> Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.
>
> The buffer memory is pre-allocated (because I don't think there is
> a precedent for working with __user memory from bpf). This might be
> slow to do for each {s,g}etsockopt call, that's why I've added
> __cgroup_bpf_prog_array_is_empty that exits early if there is nothing
> attached to a cgroup. Note, however, that there is a race between
> __cgroup_bpf_prog_array_is_empty and BPF_PROG_RUN_ARRAY where cgroup
> program layout might have changed; this should not be a problem
> because in general there is a race between multiple calls to
> {s,g}etsocktop and user adding/removing bpf progs from a cgroup.
>
> The return code of the BPF program is handled as follows:
> * 0: EPERM
> * 1: success, execute kernel {s,g}etsockopt path after BPF prog exits
> * 2: success, do _not_ execute kernel {s,g}etsockopt path after BPF
> prog exits
>
> Note that if 0 or 2 is returned from BPF program, no further BPF program
> in the cgroup hierarchy is executed. This is in contrast with any existing
> per-cgroup BPF attach_type.
This is drastically different from all other cgroup-bpf progs.
I think all programs should be executed regardless of return code.
It seems to me that 1 vs 2 difference can be expressed via bpf program logic
instead of return code.
How about we do what all other cgroup-bpf progs do:
"any no is no. all yes is yes"
Meaning any ret=0 - EPERM back to user.
If all are ret=1 - kernel handles get/set.
I think the desire to differentiate 1 vs 2 came from ordering issue
on getsockopt.
How about for setsockopt all progs run first and then kernel.
For getsockopt kernel runs first and then all progs.
Then progs will have an ability to overwrite anything the kernel returns.
^ permalink raw reply
* Re: 4.19: udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
From: Willem de Bruijn @ 2019-06-18 16:37 UTC (permalink / raw)
To: Greg KH
Cc: Naresh Kamboju, David S. Miller, Netdev, open list,
open list:KERNEL SELFTEST FRAMEWORK, Fred Klassen
In-Reply-To: <20190618161036.GA28190@kroah.com>
On Tue, Jun 18, 2019 at 12:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 18, 2019 at 08:31:16AM -0400, Willem de Bruijn wrote:
> > On Tue, Jun 18, 2019 at 7:27 AM Naresh Kamboju
> > <naresh.kamboju@linaro.org> wrote:
> > >
> > > selftests: net: udpgso_bench.sh failed on 4.19, 4.14, 4.9 and 4.4 branches.
> > > PASS on stable branch 5.1, mainline and next.
> > > This failure is started happening on 4.19 and older kernel branches after
> > > kselftest upgrade to version 5.1
> >
> > Does version 5.1 here mean running tests from Linux 5.1, against older kernels?
> >
> > > Is there any possibilities to backport ?
> > >
> > > Error:
> > > udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
> >
> > MSG_ZEROCOPY for UDP was added in commit b5947e5d1e71 ("udp:
> > msg_zerocopy") in Linux 5.0.
> >
> > The selftest was expanded with this feature in commit db63e489c7aa
> > ("selftests: extend zerocopy tests to udp"), also in Linux 5.0.
> >
> > Those tests are not expected to pass on older kernels.
>
> Any way to degrade gracefully if the feature is not present at all in
> the kernel under test? People run the latest version of kselftests on
> older kernels all the time.
We add new tests along with new features and bug fixes all the time.
All of those will fail on older kernels, as expected.
I'm honestly surprised to hear that we run newer tests against older
kernels. Is the idea to validate fixes in stable branches? If so,
should we instead backport the relevant tests to those stable
branches? Only the tests that verify fixes, leaving out those for new
features, of course.
Specific to the above test, I can add a check command testing
setsockopt SO_ZEROCOPY return value. AFAIK kselftest has no explicit
way to denote "skipped", so this would just return "pass". Sounds a
bit fragile, passing success when a feature is absent.
^ permalink raw reply
* Re: [PATCH net-next] netfilter: bridge: add nft_bridge_pvid to tag the default pvid for non-tagged packet
From: Pablo Neira Ayuso @ 2019-06-18 16:40 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1560600861-8848-1-git-send-email-wenxu@ucloud.cn>
On Sat, Jun 15, 2019 at 08:14:21PM +0800, wenxu@ucloud.cn wrote:
[...]
> +static void nft_bridge_pvid_eval(const struct nft_expr *expr,
> + struct nft_regs *regs,
> + const struct nft_pktinfo *pkt)
> +{
> + struct sk_buff *skb = pkt->skb;
> + struct net_bridge_port *p;
> +
> + p = br_port_get_rtnl_rcu(skb->dev);
> +
> + if (p && br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
> + !skb_vlan_tag_present(skb)) {
> + u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p));
> +
> + if (pvid)
> + __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, pvid);
I see two things here:
#1 Extend new NFT_META_BRIDGE_PVID nft_meta to fetch of 'pvid',
probably add net/bridge/netfilter/nft_meta_bridge.c for this.
#2 Extend nft_meta to allow to set the vlan tag via
__vlan_hwaccel_put_tag().
If these two changes are in place, then it should be possible to set
skbuff vlan id based on the pvid, if this is what you need.
This would allow for:
vlan id set bridge pvid
^ permalink raw reply
* Re: [net-next 00/11][pull request] Intel Wired LAN Driver Updates 2019-06-17
From: David Miller @ 2019-06-18 16:44 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann
In-Reply-To: <20190617233336.18119-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 17 Jun 2019 16:33:25 -0700
> This series contains updates to the iavf driver only.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] mlxsw: Improve IPv6 route insertion rate
From: David Miller @ 2019-06-18 16:46 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, dsahern, alexpe, mlxsw, idosch
In-Reply-To: <20190618151258.23023-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Tue, 18 Jun 2019 18:12:42 +0300
> Unlike IPv4, an IPv6 multipath route in the kernel is composed from
> multiple sibling routes, each representing a single nexthop.
>
> Therefore, an addition of a multipath route with N nexthops translates
> to N in-kernel notifications. This is inefficient for device drivers
> that need to program the route to the underlying device. Each time a new
> nexthop is appended, a new nexthop group needs to be constructed and the
> old one deleted.
>
> This patchset improves the situation by sending a single notification
> for a multipath route addition / deletion instead of one per-nexthop.
> When adding thousands of multipath routes with 16 nexthops, I measured
> an improvement of about x10 in the insertion rate.
...
Series applied, thanks everyone.
^ permalink raw reply
* Re: 4.19: udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
From: David Miller @ 2019-06-18 16:47 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: gregkh, naresh.kamboju, netdev, linux-kernel, linux-kselftest,
fklassen
In-Reply-To: <CAF=yD-JnTHdDE8K-EaJM2fH9awvjAmOJkoZbtU+Wi58pPnyAxw@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 18 Jun 2019 12:37:33 -0400
> Specific to the above test, I can add a check command testing
> setsockopt SO_ZEROCOPY return value. AFAIK kselftest has no explicit
> way to denote "skipped", so this would just return "pass". Sounds a
> bit fragile, passing success when a feature is absent.
Especially since the feature might be absent because the 'config'
template forgot to include a necessary Kconfig option.
^ permalink raw reply
* Re: [PATCH bpf-next v6 1/9] bpf: implement getsockopt and setsockopt hooks
From: Stanislav Fomichev @ 2019-06-18 16:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, netdev, bpf, davem, ast, daniel, Martin Lau
In-Reply-To: <20190618163117.yuw44b24lo6prsrz@ast-mbp.dhcp.thefacebook.com>
On 06/18, Alexei Starovoitov wrote:
> On Mon, Jun 17, 2019 at 11:01:01AM -0700, Stanislav Fomichev wrote:
> > Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
> > BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
> >
> > BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
> > BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
> > Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.
> >
> > The buffer memory is pre-allocated (because I don't think there is
> > a precedent for working with __user memory from bpf). This might be
> > slow to do for each {s,g}etsockopt call, that's why I've added
> > __cgroup_bpf_prog_array_is_empty that exits early if there is nothing
> > attached to a cgroup. Note, however, that there is a race between
> > __cgroup_bpf_prog_array_is_empty and BPF_PROG_RUN_ARRAY where cgroup
> > program layout might have changed; this should not be a problem
> > because in general there is a race between multiple calls to
> > {s,g}etsocktop and user adding/removing bpf progs from a cgroup.
> >
> > The return code of the BPF program is handled as follows:
> > * 0: EPERM
> > * 1: success, execute kernel {s,g}etsockopt path after BPF prog exits
> > * 2: success, do _not_ execute kernel {s,g}etsockopt path after BPF
> > prog exits
> >
> > Note that if 0 or 2 is returned from BPF program, no further BPF program
> > in the cgroup hierarchy is executed. This is in contrast with any existing
> > per-cgroup BPF attach_type.
>
> This is drastically different from all other cgroup-bpf progs.
> I think all programs should be executed regardless of return code.
> It seems to me that 1 vs 2 difference can be expressed via bpf program logic
> instead of return code.
>
> How about we do what all other cgroup-bpf progs do:
> "any no is no. all yes is yes"
> Meaning any ret=0 - EPERM back to user.
> If all are ret=1 - kernel handles get/set.
>
> I think the desire to differentiate 1 vs 2 came from ordering issue
> on getsockopt.
> How about for setsockopt all progs run first and then kernel.
> For getsockopt kernel runs first and then all progs.
> Then progs will have an ability to overwrite anything the kernel returns.
Good idea, makes sense. For getsockopt we'd also need to pass the return
value of the kernel getsockopt to let bpf programs override it, but seems
doable. Let me play with it a bit; I'll send another version if nothing
major comes up.
Thanks for another round of review!
^ 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