* Re: [PATCH v2 nf-next 1/2] netfilter: x_tables: make xt_replace_table wait until old rules are not used anymore
From: Eric Dumazet @ 2017-10-11 13:23 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Dan Williams
In-Reply-To: <20171010213945.19074-2-fw@strlen.de>
On Tue, Oct 10, 2017 at 2:39 PM, Florian Westphal <fw@strlen.de> wrote:
> xt_replace_table relies on table replacement counter retrieval (which
> uses xt_recseq to synchronize pcpu counters).
>
> This is fine, however with large rule set get_counters() can take
> a very long time -- it needs to synchronize all counters because
> it has to assume concurrent modifications can occur.
>
> Make xt_replace_table synchronize by itself by waiting until all cpus
> had an even seqcount.
>
> This allows a followup patch to copy the counters of the old ruleset
> without any synchonization after xt_replace_table has completed.
>
> Cc: Dan Williams <dcbw@redhat.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> v2: fix Erics email address
>
> net/netfilter/x_tables.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index c83a3b5e1c6c..f2d4a365768f 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -1153,6 +1153,7 @@ xt_replace_table(struct xt_table *table,
> int *error)
> {
> struct xt_table_info *private;
> + unsigned int cpu;
> int ret;
>
> ret = xt_jumpstack_alloc(newinfo);
> @@ -1184,12 +1185,20 @@ xt_replace_table(struct xt_table *table,
>
> /*
> * Even though table entries have now been swapped, other CPU's
> - * may still be using the old entries. This is okay, because
> - * resynchronization happens because of the locking done
> - * during the get_counters() routine.
> + * may still be using the old entries...
> */
> local_bh_enable();
>
> + /* ... so wait for even xt_recseq on all cpus */
> + for_each_possible_cpu(cpu) {
> + seqcount_t *s = &per_cpu(xt_recseq, cpu);
> +
> + while (raw_read_seqcount(s) & 1)
> + cpu_relax();
> +
> + cond_resched();
> + }
It seems that we could also check :
1) If low order bit of sequence is 0
Or
2) the value has changed
for_each_possible_cpu(cpu) {
seqcount_t *s = &per_cpu(xt_recseq, cpu);
u32 seq = raw_read_seqcount(s) ;
if (seq & 1) {
do {
cpu_relax();
} while (seq == raw_read_seqcount(s));
Thanks !
^ permalink raw reply
* Re: [PATCH net-next 2/2] net sched act_vlan: VLAN action rewrite to use RCU lock/unlock and update
From: Eric Dumazet @ 2017-10-11 13:30 UTC (permalink / raw)
To: Manish Kurup
Cc: jhs, xiyou.wangcong, jiri, davem, netdev, linux-kernel, aring,
mrv, manish.kurup
In-Reply-To: <1507727429.31614.22.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, 2017-10-11 at 06:10 -0700, Eric Dumazet wrote:
> On Tue, 2017-10-10 at 22:33 -0400, Manish Kurup wrote:
> > Using a spinlock in the VLAN action causes performance issues when the VLAN
> > action is used on multiple cores. Rewrote the VLAN action to use RCU read
> > locking for reads and updates instead.
> >
> > Signed-off-by: Manish Kurup <manish.kurup@verizon.com>
> > ---
> > include/net/tc_act/tc_vlan.h | 21 ++++++++-----
> > net/sched/act_vlan.c | 73 ++++++++++++++++++++++++++++++--------------
> > 2 files changed, 63 insertions(+), 31 deletions(-)
> >
> > diff --git a/include/net/tc_act/tc_vlan.h b/include/net/tc_act/tc_vlan.h
> > index c2090df..67fd355 100644
> > --- a/include/net/tc_act/tc_vlan.h
> > +++ b/include/net/tc_act/tc_vlan.h
> > @@ -13,12 +13,17 @@
> > #include <net/act_api.h>
> > #include <linux/tc_act/tc_vlan.h>
> >
> > +struct tcf_vlan_params {
> > + struct rcu_head rcu;
> > + int tcfv_action;
> > + u16 tcfv_push_vid;
> > + __be16 tcfv_push_proto;
> > + u8 tcfv_push_prio;
> > +};
> > +
> > struct tcf_vlan {
> > struct tc_action common;
> > - int tcfv_action;
> > - u16 tcfv_push_vid;
> > - __be16 tcfv_push_proto;
> > - u8 tcfv_push_prio;
> > + struct tcf_vlan_params __rcu *vlan_p;
> > };
> > #define to_vlan(a) ((struct tcf_vlan *)a)
> >
> > @@ -33,22 +38,22 @@ static inline bool is_tcf_vlan(const struct tc_action *a)
> >
> > static inline u32 tcf_vlan_action(const struct tc_action *a)
> > {
> > - return to_vlan(a)->tcfv_action;
> > + return to_vlan(a)->vlan_p->tcfv_action;
>
> This is not proper RCU : ->vlan_p should be read once, and using
> rcu_dereference()
>
> So the caller should provide to this helper the 'p' pointer instead of
> 'a'
>
>
> CONFIG_SPARSE_RCU_POINTER=y
>
> and make C=2 would probably give you warnings about that.
>
BTW no need to change your .config after :
commit 41a2901e7d220875752a8c870e0b53288a578c20
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date: Fri May 12 15:56:35 2017 -0700
rcu: Remove SPARSE_RCU_POINTER Kconfig option
The sparse-based checking for non-RCU accesses to RCU-protected pointers
has been around for a very long time, and it is now the only type of
sparse-based checking that is optional. This commit therefore makes
it unconditional.
Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Fengguang Wu <fengguang.wu@intel.com>
^ permalink raw reply
* Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()
From: Eric Dumazet @ 2017-10-11 13:31 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Hannes Frederic Sowa, Cong Wang, netdev, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau, Brendan Gregg,
Song Liu
In-Reply-To: <20171011025606.tzts5wgys5hn52p2@ast-mbp>
On Tue, 2017-10-10 at 19:56 -0700, Alexei Starovoitov wrote:
> actually we hit that too for completely different tracing use case.
> Indeed would be good to generate socket cookie unconditionally
> for all sockets. I don't think there is any harm.
>
Simply call sock_gen_cookie() when needed.
If a tracepoint needs the cookie and the cookie was not yet generated,
it will be generated at this point.
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Jes Sorensen @ 2017-10-11 13:34 UTC (permalink / raw)
To: Kalle Valo; +Cc: Gustavo A. R. Silva, linux-wireless, netdev, linux-kernel
In-Reply-To: <87o9peqdo2.fsf@kamboji.qca.qualcomm.com>
On 10/11/2017 04:41 AM, Kalle Valo wrote:
> Jes Sorensen <jes.sorensen@gmail.com> writes:
>
>> On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>> where we are expecting to fall through.
>>
>> While this isn't harmful, to me this looks like pointless patch churn
>> for zero gain and it's just ugly.
>
> In general I find it useful to mark fall through cases. And it's just a
> comment with two words, so they cannot hurt your eyes that much.
I don't see them being harmful in the code, but I don't see them of much
use either. If it happened as part of natural code development, fine. My
objection is to people running around doing this systematically causing
patch churn for little to zero gain.
Jes
^ permalink raw reply
* Re: [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier
From: Ido Schimmel @ 2017-10-11 13:37 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <1507653665-20540-3-git-send-email-dsahern@gmail.com>
On Tue, Oct 10, 2017 at 09:41:03AM -0700, David Ahern wrote:
> Add extack to in_validator_info and in6_validator_info. Update the one
> user of each, ipvlan, to return an error message for failures.
>
> Only manual configuration of an address is plumbed in the IPv6 code path.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
See two nits below. Otherwise:
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> ---
> drivers/net/ipvlan/ipvlan_main.c | 10 ++++++++--
> include/linux/inetdevice.h | 1 +
> include/net/addrconf.h | 1 +
> net/ipv4/devinet.c | 8 +++++---
> net/ipv6/addrconf.c | 23 +++++++++++++----------
> 5 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 57c3856bab05..56a868415ba2 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -846,8 +846,11 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
>
> switch (event) {
> case NETDEV_UP:
> - if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
> + if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
> + NL_SET_ERR_MSG(i6vi->extack,
> + "Address already assigned to an ipvlan device");
> return notifier_from_errno(-EADDRINUSE);
> + }
> break;
> }
>
> @@ -916,8 +919,11 @@ static int ipvlan_addr4_validator_event(struct notifier_block *unused,
>
> switch (event) {
> case NETDEV_UP:
> - if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
> + if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
> + NL_SET_ERR_MSG(ivi->extack,
> + "Address already assigned to an ipvlan device");
> return notifier_from_errno(-EADDRINUSE);
> + }
> break;
> }
>
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 751d051f0bc7..681dff30940b 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -154,6 +154,7 @@ struct in_ifaddr {
> struct in_validator_info {
> __be32 ivi_addr;
> struct in_device *ivi_dev;
> + struct netlink_ext_ack *extack;
> };
>
> int register_inetaddr_notifier(struct notifier_block *nb);
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index 87981cd63180..b8b16437c6d5 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -55,6 +55,7 @@ struct prefix_info {
> struct in6_validator_info {
> struct in6_addr i6vi_addr;
> struct inet6_dev *i6vi_dev;
> + struct netlink_ext_ack *extack;
> };
>
> #define IN6_ADDR_HSIZE_SHIFT 4
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 7ce22a2c07ce..0118698cd623 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -444,7 +444,7 @@ static void check_lifetime(struct work_struct *work);
> static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
>
> static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> - u32 portid)
> + u32 portid, struct netlink_ext_ack *extack)
Double space before `*extack`.
> {
> struct in_device *in_dev = ifa->ifa_dev;
> struct in_ifaddr *ifa1, **ifap, **last_primary;
> @@ -489,6 +489,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> */
> ivi.ivi_addr = ifa->ifa_address;
> ivi.ivi_dev = ifa->ifa_dev;
> + ivi.extack = extack;
> ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
> NETDEV_UP, &ivi);
> ret = notifier_to_errno(ret);
> @@ -521,7 +522,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
>
> static int inet_insert_ifa(struct in_ifaddr *ifa)
> {
> - return __inet_insert_ifa(ifa, NULL, 0);
> + return __inet_insert_ifa(ifa, NULL, 0, NULL);
> }
>
> static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
> @@ -902,7 +903,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
> return ret;
> }
> }
> - return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid);
> + return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
> + extack);
> } else {
> inet_free_ifa(ifa);
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 632cf4b26277..0bad4a800f73 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -958,7 +958,8 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
> static struct inet6_ifaddr *
> ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
> const struct in6_addr *peer_addr, int pfxlen,
> - int scope, u32 flags, u32 valid_lft, u32 prefered_lft)
> + int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
> + struct netlink_ext_ack *extack)
Double space before `*extack`.
> {
> struct net *net = dev_net(idev->dev);
> struct inet6_ifaddr *ifa = NULL;
> @@ -994,6 +995,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
> struct in6_validator_info i6vi = {
> .i6vi_addr = *addr,
> .i6vi_dev = idev,
> + .extack = extack,
> };
>
> rcu_read_unlock_bh();
> @@ -1336,7 +1338,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
>
> ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
> ipv6_addr_scope(&addr), addr_flags,
> - tmp_valid_lft, tmp_prefered_lft);
> + tmp_valid_lft, tmp_prefered_lft, NULL);
> if (IS_ERR(ift)) {
> in6_ifa_put(ifp);
> in6_dev_put(idev);
> @@ -2020,7 +2022,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
>
> ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
> scope, flags, valid_lft,
> - preferred_lft);
> + preferred_lft, NULL);
> if (IS_ERR(ifp2))
> goto lock_errdad;
>
> @@ -2478,7 +2480,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
> pinfo->prefix_len,
> addr_type&IPV6_ADDR_SCOPE_MASK,
> addr_flags, valid_lft,
> - prefered_lft);
> + prefered_lft, NULL);
>
> if (IS_ERR_OR_NULL(ifp))
> return -1;
> @@ -2788,7 +2790,8 @@ static int inet6_addr_add(struct net *net, int ifindex,
> const struct in6_addr *pfx,
> const struct in6_addr *peer_pfx,
> unsigned int plen, __u32 ifa_flags,
> - __u32 prefered_lft, __u32 valid_lft)
> + __u32 prefered_lft, __u32 valid_lft,
> + struct netlink_ext_ack *extack)
> {
> struct inet6_ifaddr *ifp;
> struct inet6_dev *idev;
> @@ -2847,7 +2850,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
> }
>
> ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
> - valid_lft, prefered_lft);
> + valid_lft, prefered_lft, extack);
>
> if (!IS_ERR(ifp)) {
> if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
> @@ -2932,7 +2935,7 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
> rtnl_lock();
> err = inet6_addr_add(net, ireq.ifr6_ifindex, &ireq.ifr6_addr, NULL,
> ireq.ifr6_prefixlen, IFA_F_PERMANENT,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> rtnl_unlock();
> return err;
> }
> @@ -2962,7 +2965,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
>
> ifp = ipv6_add_addr(idev, addr, NULL, plen,
> scope, IFA_F_PERMANENT,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> if (!IS_ERR(ifp)) {
> spin_lock_bh(&ifp->lock);
> ifp->flags &= ~IFA_F_TENTATIVE;
> @@ -3062,7 +3065,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
> #endif
>
> ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> if (!IS_ERR(ifp)) {
> addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
> addrconf_dad_start(ifp);
> @@ -4565,7 +4568,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
> */
> return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
> ifm->ifa_prefixlen, ifa_flags,
> - preferred_lft, valid_lft);
> + preferred_lft, valid_lft, extack);
> }
>
> if (nlh->nlmsg_flags & NLM_F_EXCL ||
> --
> 2.1.4
>
^ permalink raw reply
* Re: BUG:af_packet fails to TX TSO frames
From: Willem de Bruijn @ 2017-10-11 13:39 UTC (permalink / raw)
To: Anton Ivanov; +Cc: Network Development, David Miller
In-Reply-To: <c3002d4f-62ff-1d0b-65fb-b366be45a6d8@cambridgegreys.com>
On Wed, Oct 11, 2017 at 4:39 AM, Anton Ivanov
<anton.ivanov@cambridgegreys.com> wrote:
> Hi all,
>
> I am having an issue with af_packet.c
>
> It fails to transmit any TSO frame submitted via raw socket + vnet headers.
> An identical frame is considered valid for tap.
This may be due to validation. As of commit 104ba78c9880 ("packet: on
direct_xmit, limit tso and csum to supported devices") the packet socket
code validates TSO packets and HW support in the direct_xmit path.
Do you have a test program or packet (incl. vnet hdr) to reproduce this
with? I usually test this path with
https://github.com/wdebruij/kerneltools/blob/master/tests/psock_txring_vnet.c
> The frames are generated out of legit linux skbufs (in UML) and vnet headers
> work for checksumming on raw, so I should have the raw initialization right.
>
> The header is supposedly parsed correctly and the newly formed skbuf is sent
> to the device transmit routine (or enqueued) . I have debugged it as far as
> it reaching the following line in packet_snd() (line 2592 in 4.13):
>
> err = po->xmit(skb);
That maps either on to packet_direct_xmit or dev_queue_xmit.
>
> This returns NET_XMIT_DROP for any TSO capable device I tested.
You can also try
perf record -a -g -e skb:kfree_skb sleep 10
perf report
to see where these packets are dropped.
> They dislike
> the frame. Same frame is accepted by tap. I have went through the header
> parsing and skb allocation code in both af_packet and tap several times and
> I do not see any material difference (except the new zerocopy stuff). So,
> frankly, I am stuck.
>
> Can someone help me to debug this. I do not see an easy way to debug it, but
> this is not a part of the kernel I am familiar with. Is there a suitable
> helper function to try to segment the frame and see exactly what is wrong
> with it?
>
> Cc-ing DaveM as this has no specific maintainer so it falls under his
> umbrella remit.
>
> --
> Anton R. Ivanov
>
> Cambridge Greys Limited, England and Wales company No 10273661
> http://www.cambridgegreys.com/
>
^ permalink raw reply
* Re: [PATCH v2 nf-next 1/2] netfilter: x_tables: make xt_replace_table wait until old rules are not used anymore
From: Florian Westphal @ 2017-10-11 13:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev, Dan Williams
In-Reply-To: <CANn89i+Y9CtNq6qTGAWi7fNfk9RRyxsuY1e2+SBCd1xRYx77zw@mail.gmail.com>
Eric Dumazet <edumazet@google.com> wrote:
> > + /* ... so wait for even xt_recseq on all cpus */
> > + for_each_possible_cpu(cpu) {
> > + seqcount_t *s = &per_cpu(xt_recseq, cpu);
> > +
> > + while (raw_read_seqcount(s) & 1)
> > + cpu_relax();
> > +
> > + cond_resched();
> > + }
>
> It seems that we could also check :
>
> 1) If low order bit of sequence is 0
>
> Or
>
> 2) the value has changed
> for_each_possible_cpu(cpu) {
> seqcount_t *s = &per_cpu(xt_recseq, cpu);
> u32 seq = raw_read_seqcount(s) ;
>
> if (seq & 1) {
> do {
> cpu_relax();
> } while (seq == raw_read_seqcount(s));
>
Actually I first used
for_each_possible_cpu(cpu) {
seqcount_t *s = &per_cpu(xt_recseq, cpu);
(void)__read_seqcount_begin(s);
}
but it looked confusing (not paired with a _retry function).
I'll respin with a loop like your suggestion above. Thanks!
^ permalink raw reply
* Re: BUG:af_packet fails to TX TSO frames
From: Anton Ivanov @ 2017-10-11 13:50 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, David Miller
In-Reply-To: <CAF=yD-+SxQTHxiU5Yi2f8dm6X1s7SA6PdkbeJP-K_9ZuTMAaTA@mail.gmail.com>
On 10/11/17 14:39, Willem de Bruijn wrote:
> On Wed, Oct 11, 2017 at 4:39 AM, Anton Ivanov
> <anton.ivanov@cambridgegreys.com> wrote:
>> Hi all,
>>
>> I am having an issue with af_packet.c
>>
>> It fails to transmit any TSO frame submitted via raw socket + vnet headers.
>> An identical frame is considered valid for tap.
> This may be due to validation. As of commit 104ba78c9880 ("packet: on
> direct_xmit, limit tso and csum to supported devices") the packet socket
> code validates TSO packets and HW support in the direct_xmit path.
I will look at it. I have tried with bridge+vEth (raw socket on the
vEth) and directly on a tg3 and e1000e. All of these should be tso capable.
CSUM definitely works in both cases, it is only TSO and only via raw
socket on TX which is refusing to work.
>
> Do you have a test program or packet (incl. vnet hdr) to reproduce this
> with? I usually test this path with
My test program at present is UML instance with my vector IO patches.
Latest version just went to the uml mailing list and can be pulled from
http://foswiki.kot-begemot.co.uk/Main/EatYourOwnDogFoodOnUML
I am going to dump one of the frames being produced and isolate it for a
standalone test case.
>
> https://github.com/wdebruij/kerneltools/blob/master/tests/psock_txring_vnet.c
>
>> The frames are generated out of legit linux skbufs (in UML) and vnet headers
>> work for checksumming on raw, so I should have the raw initialization right.
>>
>> The header is supposedly parsed correctly and the newly formed skbuf is sent
>> to the device transmit routine (or enqueued) . I have debugged it as far as
>> it reaching the following line in packet_snd() (line 2592 in 4.13):
>>
>> err = po->xmit(skb);
> That maps either on to packet_direct_xmit or dev_queue_xmit.
I know. In my case it is direct_xmit as I have asked for QDISC bypass.
>
>> This returns NET_XMIT_DROP for any TSO capable device I tested.
> You can also try
>
> perf record -a -g -e skb:kfree_skb sleep 10
> perf report
>
> to see where these packets are dropped.
Thanks, will try that.
>
>> They dislike
>> the frame. Same frame is accepted by tap. I have went through the header
>> parsing and skb allocation code in both af_packet and tap several times and
>> I do not see any material difference (except the new zerocopy stuff). So,
>> frankly, I am stuck.
>>
>> Can someone help me to debug this. I do not see an easy way to debug it, but
>> this is not a part of the kernel I am familiar with. Is there a suitable
>> helper function to try to segment the frame and see exactly what is wrong
>> with it?
>>
>> Cc-ing DaveM as this has no specific maintainer so it falls under his
>> umbrella remit.
>>
>> --
>> Anton R. Ivanov
>>
>> Cambridge Greys Limited, England and Wales company No 10273661
>> http://www.cambridgegreys.com/
>>
--
Anton R. Ivanov
Cambridge Greys Limited, England and Wales company No 10273661
http://www.cambridgegreys.com/
^ permalink raw reply
* Re: [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier
From: Ido Schimmel @ 2017-10-11 13:54 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <1507653665-20540-4-git-send-email-dsahern@gmail.com>
On Tue, Oct 10, 2017 at 09:41:04AM -0700, David Ahern wrote:
> Add support for inetaddr_validator and inet6addr_validator. The
> notifiers provide a means for validating ipv4 and ipv6 addresses
> before the addresses are installed and on failure the error
> is propagated back to the user.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 10 ++++
> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 53 ++++++++++++++++++++++
> 3 files changed, 67 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index 321988ac57cc..da4ee91235be 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -4505,11 +4505,19 @@ static struct notifier_block mlxsw_sp_netdevice_nb __read_mostly = {
> .notifier_call = mlxsw_sp_netdevice_event,
> };
>
> +static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
> + .notifier_call = mlxsw_sp_inetaddr_valid_event,
> +};
> +
> static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
> .notifier_call = mlxsw_sp_inetaddr_event,
> .priority = 10, /* Must be called before FIB notifier block */
This line can now be removed since the validator notifier is called
before the inetaddr notifier.
It's used to create a RIF before fib_add_ifaddr() is invoked to create a
prefix route. IPv6 doesn't need that since routes aren't created based
on a NETDEV_UP event in inet6addr.
> };
>
> +static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
> + .notifier_call = mlxsw_sp_inet6addr_valid_event,
> +};
> +
> static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
> .notifier_call = mlxsw_sp_inet6addr_event,
> };
> @@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
> int err;
>
> register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
> + register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
> register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
> + register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
> register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
> register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
Need unregister in rollback and __exit function.
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index 8e45183dc9bb..4865a6f58c83 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
> int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
> int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
> unsigned long event, void *ptr);
> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr);
> int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> unsigned long event, void *ptr);
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr);
> int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
> struct netdev_notifier_changeupper_info *info);
> void
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 6a356f4b99a3..7d53fdf2c0a8 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5656,6 +5656,33 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
> struct mlxsw_sp_rif *rif;
> int err = 0;
>
> + /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
> + if (event == NETDEV_UP)
> + goto out;
> +
> + mlxsw_sp = mlxsw_sp_lower_get(dev);
> + if (!mlxsw_sp)
> + goto out;
> +
> + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> + if (!mlxsw_sp_rif_should_config(rif, dev, event))
> + goto out;
> +
> + err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> + return notifier_from_errno(err);
> +}
> +
> +/* only expected to be called for event == NETDEV_UP */
I don't really mind, but I don't think the comment is necessary.
> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct in_validator_info *ivi = (struct in_validator_info *)ptr;
We usually put a space after the cast in mlxsw (I'm aware of the
checkpatch CHECK).
> + struct net_device *dev = ivi->ivi_dev->dev;
> + struct mlxsw_sp *mlxsw_sp;
> + struct mlxsw_sp_rif *rif;
> + int err = 0;
> +
> mlxsw_sp = mlxsw_sp_lower_get(dev);
> if (!mlxsw_sp)
> goto out;
> @@ -5708,6 +5735,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
> struct net_device *dev = if6->idev->dev;
>
> + /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
> + if (event == NETDEV_UP)
> + return NOTIFY_DONE;
> +
> if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
> return NOTIFY_DONE;
>
> @@ -5724,6 +5755,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> return NOTIFY_DONE;
> }
>
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
Same as above.
Looks good otherwise, thanks!
> + struct net_device *dev = i6vi->i6vi_dev->dev;
> + struct mlxsw_sp *mlxsw_sp;
> + struct mlxsw_sp_rif *rif;
> + int err = 0;
> +
> + mlxsw_sp = mlxsw_sp_lower_get(dev);
> + if (!mlxsw_sp)
> + goto out;
> +
> + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> + if (!mlxsw_sp_rif_should_config(rif, dev, event))
> + goto out;
> +
> + err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> + return notifier_from_errno(err);
> +}
> +
> static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
> const char *mac, int mtu)
> {
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH] [net-next] ip_tunnel: fix building with NET_IP_TUNNEL=m
From: Arnd Bergmann @ 2017-10-11 13:55 UTC (permalink / raw)
To: David S. Miller; +Cc: Arnd Bergmann, netdev, linux-kernel
When af_mpls is built-in but the tunnel support is a module,
we get a link failure:
net/mpls/af_mpls.o: In function `mpls_init':
af_mpls.c:(.init.text+0xdc): undefined reference to `ip_tunnel_encap_add_ops'
This adds a Kconfig statement to prevent the broken
configuration and force mpls to be a module as well in
this case.
Fixes: bdc476413dcd ("ip_tunnel: add mpls over gre support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/mpls/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig
index 5c467ef97311..801ea9098387 100644
--- a/net/mpls/Kconfig
+++ b/net/mpls/Kconfig
@@ -24,6 +24,7 @@ config NET_MPLS_GSO
config MPLS_ROUTING
tristate "MPLS: routing support"
+ depends on NET_IP_TUNNEL || NET_IP_TUNNEL=n
---help---
Add support for forwarding of mpls packets.
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] [net-next] ip_tunnel: fix building with NET_IP_TUNNEL=m
From: Arnd Bergmann @ 2017-10-11 13:57 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Networking, Linux Kernel Mailing List,
Amine Kherbouche
In-Reply-To: <20171011135546.3536829-1-arnd@arndb.de>
I forgot to Cc Amine, sorry.
On Wed, Oct 11, 2017 at 3:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> When af_mpls is built-in but the tunnel support is a module,
> we get a link failure:
>
> net/mpls/af_mpls.o: In function `mpls_init':
> af_mpls.c:(.init.text+0xdc): undefined reference to `ip_tunnel_encap_add_ops'
>
> This adds a Kconfig statement to prevent the broken
> configuration and force mpls to be a module as well in
> this case.
>
> Fixes: bdc476413dcd ("ip_tunnel: add mpls over gre support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/mpls/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig
> index 5c467ef97311..801ea9098387 100644
> --- a/net/mpls/Kconfig
> +++ b/net/mpls/Kconfig
> @@ -24,6 +24,7 @@ config NET_MPLS_GSO
>
> config MPLS_ROUTING
> tristate "MPLS: routing support"
> + depends on NET_IP_TUNNEL || NET_IP_TUNNEL=n
> ---help---
> Add support for forwarding of mpls packets.
>
> --
> 2.9.0
>
^ permalink raw reply
* [PATCH] fm10k: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2017-10-11 13:58 UTC (permalink / raw)
To: Jeff Kirsher
Cc: Arnd Bergmann, Jacob Keller, Ngai-Mint Kwan, David S. Miller,
Florian Westphal, intel-wired-lan, netdev, linux-kernel
A cleanup of the PM code left an incorrect #ifdef in place, leading
to a harmless build warning:
drivers/net/ethernet/intel/fm10k/fm10k_pci.c:2502:12: error: 'fm10k_suspend' defined but not used [-Werror=unused-function]
drivers/net/ethernet/intel/fm10k/fm10k_pci.c:2475:12: error: 'fm10k_resume' defined but not used [-Werror=unused-function]
It's easier to use __maybe_unused attributes here, since you
can't pick the wrong one.
Fixes: 8249c47c6ba4 ("fm10k: use generic PM hooks instead of legacy PCIe power hooks")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 1e9ae3197b17..52f8eb3c470e 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -2463,7 +2463,6 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)
return err;
}
-#ifdef CONFIG_PM
/**
* fm10k_resume - Generic PM resume hook
* @dev: generic device structure
@@ -2472,7 +2471,7 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)
* suspend or hibernation. This function does not need to handle lower PCIe
* device state as the stack takes care of that for us.
**/
-static int fm10k_resume(struct device *dev)
+static int __maybe_unused fm10k_resume(struct device *dev)
{
struct fm10k_intfc *interface = pci_get_drvdata(to_pci_dev(dev));
struct net_device *netdev = interface->netdev;
@@ -2499,7 +2498,7 @@ static int fm10k_resume(struct device *dev)
* system suspend or hibernation. This function does not need to handle lower
* PCIe device state as the stack takes care of that for us.
**/
-static int fm10k_suspend(struct device *dev)
+static int __maybe_unused fm10k_suspend(struct device *dev)
{
struct fm10k_intfc *interface = pci_get_drvdata(to_pci_dev(dev));
struct net_device *netdev = interface->netdev;
@@ -2511,8 +2510,6 @@ static int fm10k_suspend(struct device *dev)
return 0;
}
-#endif /* CONFIG_PM */
-
/**
* fm10k_io_error_detected - called when PCI error is detected
* @pdev: Pointer to PCI device
@@ -2643,11 +2640,9 @@ static struct pci_driver fm10k_driver = {
.id_table = fm10k_pci_tbl,
.probe = fm10k_probe,
.remove = fm10k_remove,
-#ifdef CONFIG_PM
.driver = {
.pm = &fm10k_pm_ops,
},
-#endif /* CONFIG_PM */
.sriov_configure = fm10k_iov_configure,
.err_handler = &fm10k_err_handler
};
--
2.9.0
^ permalink raw reply related
* RE: [PATCH v2 net-next 1/2] net: dsa: lan9303: Move tag setup to new lan9303_setup_tagging
From: Woojung.Huh @ 2017-10-11 14:01 UTC (permalink / raw)
To: privat, andrew, vivien.didelot, f.fainelli, netdev, linux-kernel
In-Reply-To: <20f8c457-0f61-641e-a12a-7c0bcf2fba21@egil-hjelmeland.no>
> >>>> @@ -644,6 +648,10 @@ static int lan9303_setup(struct dsa_switch *ds)
> >>>> return -EINVAL;
> >>>> }
> >>>>
> >>>> + ret = lan9303_setup_tagging(chip);
> >>>> + if (ret)
> >>>> + dev_err(chip->dev, "failed to setup port tagging %d\n", ret);
> >>>> +
> >>> Still move on when error happens?
> >>>
> >> Good question. I just followed the pattern from the original function,
> >> which was not made by me. Actually I did once reflect on whether this
> >> was the correct way. Perhaps it could be argued that it is better to
> >> allow the device to come up, so the problem can be investigated?
> > Maybe depends on severity of setting?
> > BTW, lan9303_setup() still returns ZERO at the end?
> I did quick survey of the _setup functions of the other dsa drivers.
> Some return on error, some ignore errors.
> If you think so, I can make a v3 series that return on error. Otherwise
> I leave it as it is.
Unless Andrew, Vivien or Florian raises flag, I guess it will be fine as-is.
Thanks.
Woojung
^ permalink raw reply
* [PATCH] i40e/i40evf: actually use u32 for feature flags
From: Arnd Bergmann @ 2017-10-11 14:02 UTC (permalink / raw)
To: Jeff Kirsher
Cc: Arnd Bergmann, Jacob Keller, Alexander Duyck, Mitch Williams,
Filip Sadowski, intel-wired-lan, netdev, linux-kernel
A previous cleanup intended to change the flags variable to 32
bit instead of 64, but accidentally left out the important
part of that change, leading to a build error:
drivers/net/ethernet/intel/i40e/i40e_ethtool.o: In function `i40e_set_priv_flags':
i40e_ethtool.c:(.text+0x1a94): undefined reference to `wrong_size_cmpxchg'
This adds the missing modification.
Fixes: b74f571f59a8 ("i40e/i40evf: organize and re-number feature flags")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 18c453a3e728..7baf6d8a84dd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -424,7 +424,7 @@ struct i40e_pf {
#define I40E_HW_PORT_ID_VALID BIT(17)
#define I40E_HW_RESTART_AUTONEG BIT(18)
- u64 flags;
+ u32 flags;
#define I40E_FLAG_RX_CSUM_ENABLED BIT(0)
#define I40E_FLAG_MSI_ENABLED BIT(1)
#define I40E_FLAG_MSIX_ENABLED BIT(2)
--
2.9.0
^ permalink raw reply related
* Re: [PATCH v3] mac80211: aead api to reduce redundancy
From: Xiang Gao @ 2017-10-11 14:12 UTC (permalink / raw)
To: Johannes Berg; +Cc: David S. Miller, linux-kernel, linux-wireless, netdev
In-Reply-To: <1507708082.1998.9.camel@sipsolutions.net>
Great ! Thanks !
Xiang Gao
2017-10-11 3:48 GMT-04:00 Johannes Berg <johannes@sipsolutions.net>:
> On Tue, 2017-10-10 at 22:31 -0400, Xiang Gao wrote:
>> Currently, the aes_ccm.c and aes_gcm.c are almost line by line copy
>> of
>> each other. This patch reduce code redundancy by moving the code in
>> these
>> two files to crypto/aead_api.c to make it a higher level aead api.
>> The
>> file aes_ccm.c and aes_gcm.c are removed and all the functions there
>> are
>> now implemented in their headers using the newly added aead api.
>
> Applied. FWIW, the sta_dynamic_down_up test that the robot complained
> about, and the PSK tests that heavily use CCMP all pass.
>
> johannes
^ permalink raw reply
* Re: [PATCH][net-next] net: mpls: make function ipgre_mpls_encap_hlen static
From: David Ahern @ 2017-10-11 14:12 UTC (permalink / raw)
To: Colin King, David S . Miller, Robert Shearman, Roopa Prabhu,
netdev
Cc: kernel-janitors, linux-kernel, Amine Kherbouche
In-Reply-To: <20171011095328.17546-1-colin.king@canonical.com>
On 10/11/17 3:53 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The function ipgre_mpls_encap_hlen is local to the source and
> does not need to be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol 'ipgre_mpls_encap_hlen' was not declared. Should it be static?
>
Fixes: bdc476413dcdb ("ip_tunnel: add mpls over gre support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/mpls/af_mpls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index 9745e8f69810..8ca9915befc8 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -41,7 +41,7 @@ static int label_limit = (1 << 20) - 1;
> static int ttl_max = 255;
>
> #if IS_ENABLED(CONFIG_NET_IP_TUNNEL)
> -size_t ipgre_mpls_encap_hlen(struct ip_tunnel_encap *e)
> +static size_t ipgre_mpls_encap_hlen(struct ip_tunnel_encap *e)
> {
> return sizeof(struct mpls_shim_hdr);
> }
>
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
From: Ido Schimmel @ 2017-10-11 14:13 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
In-Reply-To: <1507653665-20540-5-git-send-email-dsahern@gmail.com>
On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
> Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
> to set an error message on RIF or VR overflow. Now an overflow of
> either resource the use gets an informative message as opposed to
s/the/in/ ?
> failing with EBUSY.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
One comment below, but other than that:
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
I'll run some tests and report if anything blows up.
Thanks David!
> ---
> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 114 +++++++++++++--------
> 1 file changed, 69 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 7d53fdf2c0a8..ec4d313b9eca 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
> }
>
> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
> - u32 tb_id)
> + u32 tb_id,
> + struct netlink_ext_ack *extack)
> {
> struct mlxsw_sp_vr *vr;
> int err;
>
> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
> - if (!vr)
> + if (!vr) {
> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
Maybe:
"spectrum: Exceeded number of supported VRF devices"
To be consistent with previously added:
"spectrum: Exceeded number of supported LAG devices"
> return ERR_PTR(-EBUSY);
> + }
> vr->fib4 = mlxsw_sp_fib_create(vr, MLXSW_SP_L3_PROTO_IPV4);
> if (IS_ERR(vr->fib4))
> return ERR_CAST(vr->fib4);
> @@ -775,14 +778,15 @@ static void mlxsw_sp_vr_destroy(struct mlxsw_sp_vr *vr)
> vr->fib4 = NULL;
> }
^ permalink raw reply
* Re: [PATCH net v2] net: enable interface alias removal via rtnl
From: David Ahern @ 2017-10-11 14:13 UTC (permalink / raw)
To: nicolas.dichtel, davem; +Cc: netdev, oliver, Stephen Hemminger
In-Reply-To: <8b334f53-235b-42d2-ba68-4e63c54c498c@6wind.com>
On 10/11/17 6:29 AM, Nicolas Dichtel wrote:
> Le 10/10/2017 à 16:50, David Ahern a écrit :
>> On 10/10/17 6:41 AM, Nicolas Dichtel wrote:
>>> IFLA_IFALIAS is defined as NLA_STRING. It means that the minimal length of
>>> the attribute is 1 ("\0"). However, to remove an alias, the attribute
>>> length must be 0 (see dev_set_alias()).
>>>
>>> Let's define the type to NLA_BINARY, so that the alias can be removed.
>>
>> not to be pedantic, but we need to be clear that the type is changed
>> only for policy validation.
> With the comment in the code, it is clear, isn't it?
Code comment was fine; commit log -- line referenced above -- is open
for interpretation.
^ permalink raw reply
* [PATCH net v3] net: enable interface alias removal via rtnl
From: Nicolas Dichtel @ 2017-10-11 14:24 UTC (permalink / raw)
To: dsahern; +Cc: davem, netdev, oliver, Nicolas Dichtel, Stephen Hemminger
In-Reply-To: <66bbef72-90fb-7142-2070-07dc0c2b0ca9@gmail.com>
IFLA_IFALIAS is defined as NLA_STRING. It means that the minimal length of
the attribute is 1 ("\0"). However, to remove an alias, the attribute
length must be 0 (see dev_set_alias()).
Let's define the type to NLA_BINARY to allow 0-length string, so that the
alias can be removed.
Example:
$ ip l s dummy0 alias foo
$ ip l l dev dummy0
5: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether ae:20:30:4f:a7:f3 brd ff:ff:ff:ff:ff:ff
alias foo
Before the patch:
$ ip l s dummy0 alias ""
RTNETLINK answers: Numerical result out of range
After the patch:
$ ip l s dummy0 alias ""
$ ip l l dev dummy0
5: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether ae:20:30:4f:a7:f3 brd ff:ff:ff:ff:ff:ff
CC: Oliver Hartkopp <oliver@hartkopp.net>
CC: Stephen Hemminger <stephen@networkplumber.org>
Fixes: 96ca4a2cc145 ("net: remove ifalias on empty given alias")
Reported-by: Julien FLoret <julien.floret@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
David A., I hope that it is now clear and that a v4 will not be needed
for a so trivial patch.
v2 -> v3: reword the commit log
v1 -> v2: add the comment
net/core/rtnetlink.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d4bcdcc68e92..5343565d88b7 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1483,7 +1483,10 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_LINKINFO] = { .type = NLA_NESTED },
[IFLA_NET_NS_PID] = { .type = NLA_U32 },
[IFLA_NET_NS_FD] = { .type = NLA_U32 },
- [IFLA_IFALIAS] = { .type = NLA_STRING, .len = IFALIASZ-1 },
+ /* IFLA_IFALIAS is a string, but policy is set to NLA_BINARY to
+ * allow 0-length string (needed to remove an alias).
+ */
+ [IFLA_IFALIAS] = { .type = NLA_BINARY, .len = IFALIASZ - 1 },
[IFLA_VFINFO_LIST] = {. type = NLA_NESTED },
[IFLA_VF_PORTS] = { .type = NLA_NESTED },
[IFLA_PORT_SELF] = { .type = NLA_NESTED },
--
2.13.2
^ permalink raw reply related
* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-11 14:32 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel, Kees Cook
In-Reply-To: <eb89f2ad-f2d5-b97f-c224-daf3953d912c@gmail.com>
Hi Jes,
Quoting Jes Sorensen <jes.sorensen@gmail.com>:
> On 10/11/2017 04:41 AM, Kalle Valo wrote:
>> Jes Sorensen <jes.sorensen@gmail.com> writes:
>>
>>> On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
>>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>>> where we are expecting to fall through.
>>>
>>> While this isn't harmful, to me this looks like pointless patch churn
>>> for zero gain and it's just ugly.
>>
>> In general I find it useful to mark fall through cases. And it's just a
>> comment with two words, so they cannot hurt your eyes that much.
>
> I don't see them being harmful in the code, but I don't see them of
> much use either. If it happened as part of natural code development,
> fine. My objection is to people running around doing this
> systematically causing patch churn for little to zero gain.
>
> Jes
I understand that you think this is of zero gain for you, but as
Florian Fainelli pointed out:
"That is the canonical way to tell static analyzers and compilers that
fall throughs are wanted and not accidental mistakes in the code. For
people that deal with these kinds of errors, it's quite helpful, unless
you suggest disabling that particular GCC warning specific for that
file/directory?"
this is very helpful for people working on fixing issues reported by
static analyzers. It saves a huge amount of time when dealing with
False Positives. Also, there are cases when an apparently intentional
fall-through turns out to be an actual missing break or continue.
So there is an ongoing effort to detect such cases and avoid them to
show up in the future by at least warning people about a potential
issue in their code. And this is helpful for everybody.
Thanks
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [PATCHv4 iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: David Ahern @ 2017-10-11 14:35 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger, Michal Kubecek, Hangbin Liu,
netdev, Hangbin Liu
In-Reply-To: <20171011111007.GA11332@orbyte.nwl.cc>
On 10/11/17 5:10 AM, Phil Sutter wrote:
> less than 10% overall. Given the short time a single call to 'ip'
> typically takes, I don't think the difference is noticeable even in
> highly performance critical applications.
high performance critical applications should not be forking ip and
definitely not for 1 command at a time.
^ permalink raw reply
* RE: [PATCH v2 net 1/2] net/smc: replace function pointer get_netdev()
From: Parav Pandit @ 2017-10-11 14:41 UTC (permalink / raw)
To: Ursula Braun, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
In-Reply-To: <20171011114723.30733-2-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> -----Original Message-----
> From: Ursula Braun [mailto:ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org]
> Sent: Wednesday, October 11, 2017 6:47 AM
> To: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org; schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org;
> heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org; raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org;
> ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org; Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: [PATCH v2 net 1/2] net/smc: replace function pointer get_netdev()
>
> SMC should not open code the function pointer get_netdev of the IB device.
> Replacing ib_query_gid(..., NULL) with ib_query_gid(..., gid_attr) allows access
> to the netdev.
>
> Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Suggested-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Looks fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v2 net 2/2] net/smc: dev_put for netdev after usage of ib_query_gid()
From: Parav Pandit @ 2017-10-11 14:43 UTC (permalink / raw)
To: Ursula Braun, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-s390@vger.kernel.org, jwi@linux.vnet.ibm.com,
schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com,
raspl@linux.vnet.ibm.com
In-Reply-To: <20171011114723.30733-3-ubraun@linux.vnet.ibm.com>
> -----Original Message-----
> From: Ursula Braun [mailto:ubraun@linux.vnet.ibm.com]
> Sent: Wednesday, October 11, 2017 6:47 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; linux-
> s390@vger.kernel.org; jwi@linux.vnet.ibm.com; schwidefsky@de.ibm.com;
> heiko.carstens@de.ibm.com; raspl@linux.vnet.ibm.com;
> ubraun@linux.vnet.ibm.com; Parav Pandit <parav@mellanox.com>
> Subject: [PATCH v2 net 2/2] net/smc: dev_put for netdev after usage of
> ib_query_gid()
>
> For RoCEs ib_query_gid() takes a reference count on the net_device.
> This reference count must be decreased by the caller.
>
> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
> Reported-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Fixes: 0cfdd8f92cac ("smc: connection and link group creation")
> ---
> net/smc/smc_core.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c index
> 20b66e79c5d6..5f6a20084157 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -380,10 +380,14 @@ static int smc_link_determine_gid(struct
> smc_link_group *lgr)
> if (ib_query_gid(lnk->smcibdev->ibdev, lnk->ibport, i, &gid,
> &gattr))
> continue;
> - if (gattr.ndev &&
> - (vlan_dev_vlan_id(gattr.ndev) == lgr->vlan_id)) {
> - lnk->gid = gid;
> - return 0;
> + if (gattr.ndev) {
> + if (is_vlan_dev(gattr.ndev) &&
> + vlan_dev_vlan_id(gattr.ndev) == lgr->vlan_id) {
> + lnk->gid = gid;
> + dev_put(gattr.ndev);
> + return 0;
> + }
> + dev_put(gattr.ndev);
> }
> }
> return -ENODEV;
> --
> 2.13.5
Looks fine now.
^ permalink raw reply
* Re: [PATCH 0/4] RCU: introduce noref debug
From: Paolo Abeni @ 2017-10-11 14:50 UTC (permalink / raw)
To: paulmck
Cc: linux-kernel, Josh Triplett, Steven Rostedt, David S. Miller,
Eric Dumazet, Hannes Frederic Sowa, netdev
In-Reply-To: <20171011040225.GU3521@linux.vnet.ibm.com>
On Tue, 2017-10-10 at 21:02 -0700, Paul E. McKenney wrote:
> Linus and Ingo will ask me how users decide how they should set that
> additional build flag. Especially given that if there is code that
> requires non-strict checking, isn't everyone required to set up non-strict
> checking to avoid false positives? Unless you can also configure out
> all the code that requires non-strict checking, I suppose, but how
> would you keep track of what to configure out?
I'm working to a new version using a single compile flag - without
additional strict option.
I don't know of any other subsytem that stores rcu pointer in
datastructures for a longer amount of time. That having said, I wonder
if the tests should completely move to the networking subsystem for the
time being. The Kconfig option would thus be called NET_DEBUG or
something along the lines. For abstraction it would be possible to add
an atomic_notifier_chain to the rcu_read/unlock methods, where multiple
users or checkers could register for. That way we keep the users
seperate from the implementation for the cost of a bit more layering
and more indirect calls. But given that this will anyway slow down
execution a lot, it will anyway only be suitable in
testing/verification/debugging environments.
> OK. There will probably be some discussion about the API in that case.
I'll drop noref parameter, the key will became mandatory - the exact
position of where the reference of RCU managed object is stored. In the
case of noref dst it is &skb->_skb_refdst. With this kind of API it
should suite more subsystems.
> True enough. Except that if people were good about always clearing the
> pointer, then the pointer couldn't leak, right? Or am I missing something
> in your use cases?
This is correct. The dst_entry checking in skb, which this patch series
implements there are strict brackets in the sense of skb_dst_set,
skb_dst_set_noref, skb_dst_force, etc., which form brackets around the
safe uses of those dst_entries. This patch series validates that the
correct skb_dst_* functions are being called before the sk_buff leaves
the rcu protected section. Thus we don't need to modify and review a
lot of code but we can just patch into those helpers already.
> Or to put it another way -- have you been able to catch any real
> pointer-leak bugs with thister-leak bugs with this? The other RCU
> debug options have had pretty long found-bug lists before we accepted
> them.
There have been two problems found so far, one is a rather minor one
while the other one seems like a normal bug. The patches for those are
part of this series (3/4 and 4/4).
Regards,
Paolo
^ permalink raw reply
* [PATCH net-next 0/2] Fix IFE meta modules loading
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
Adjust module alias names of IFE meta modules and fix the bug that
prevented auto-loading IFE modules in run-time.
Roman Mashak (2):
net sched actions: change IFE modules alias names
net sched actions: fix module auto-loading
include/net/tc_act/tc_ife.h | 2 +-
net/sched/act_ife.c | 16 +++++++++++++++-
net/sched/act_meta_mark.c | 2 +-
net/sched/act_meta_skbprio.c | 2 +-
net/sched/act_meta_skbtcindex.c | 2 +-
5 files changed, 19 insertions(+), 5 deletions(-)
--
1.9.1
^ 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