* 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
* [PATCH net-next 1/2] net sched actions: change IFE modules alias names
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1507733430-17860-1-git-send-email-mrv@mojatatu.com>
Make style of module alias name consistent with other subsystems in kernel,
for example net devices.
Fixes: 084e2f6566d2 ("Support to encoding decoding skb mark on IFE action")
Fixes: 200e10f46936 ("Support to encoding decoding skb prio on IFE action")
Fixes: 408fbc22ef1e ("net sched ife action: Introduce skb tcindex metadata encap decap")
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
include/net/tc_act/tc_ife.h | 2 +-
net/sched/act_ife.c | 2 +-
net/sched/act_meta_mark.c | 2 +-
net/sched/act_meta_skbprio.c | 2 +-
net/sched/act_meta_skbtcindex.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/tc_act/tc_ife.h b/include/net/tc_act/tc_ife.h
index 30ba459..104578f 100644
--- a/include/net/tc_act/tc_ife.h
+++ b/include/net/tc_act/tc_ife.h
@@ -40,7 +40,7 @@ struct tcf_meta_ops {
struct module *owner;
};
-#define MODULE_ALIAS_IFE_META(metan) MODULE_ALIAS("ifemeta" __stringify_1(metan))
+#define MODULE_ALIAS_IFE_META(metan) MODULE_ALIAS("ife-meta-" metan)
int ife_get_meta_u32(struct sk_buff *skb, struct tcf_meta_info *mi);
int ife_get_meta_u16(struct sk_buff *skb, struct tcf_meta_info *mi);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 8ccd358..791aeee 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -263,7 +263,7 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
if (exists)
spin_unlock_bh(&ife->tcf_lock);
rtnl_unlock();
- request_module("ifemeta%u", metaid);
+ request_module("ife-meta-%u", metaid);
rtnl_lock();
if (exists)
spin_lock_bh(&ife->tcf_lock);
diff --git a/net/sched/act_meta_mark.c b/net/sched/act_meta_mark.c
index 8289217..1e3f10e 100644
--- a/net/sched/act_meta_mark.c
+++ b/net/sched/act_meta_mark.c
@@ -76,4 +76,4 @@ static void __exit ifemark_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2015)");
MODULE_DESCRIPTION("Inter-FE skb mark metadata module");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_SKBMARK);
+MODULE_ALIAS_IFE_META("skbmark");
diff --git a/net/sched/act_meta_skbprio.c b/net/sched/act_meta_skbprio.c
index 26bf4d8..4033f9f 100644
--- a/net/sched/act_meta_skbprio.c
+++ b/net/sched/act_meta_skbprio.c
@@ -73,4 +73,4 @@ static void __exit ifeprio_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2015)");
MODULE_DESCRIPTION("Inter-FE skb prio metadata action");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_PRIO);
+MODULE_ALIAS_IFE_META("skbprio");
diff --git a/net/sched/act_meta_skbtcindex.c b/net/sched/act_meta_skbtcindex.c
index 3b35774..2ea1f26 100644
--- a/net/sched/act_meta_skbtcindex.c
+++ b/net/sched/act_meta_skbtcindex.c
@@ -76,4 +76,4 @@ static void __exit ifetc_index_cleanup_module(void)
MODULE_AUTHOR("Jamal Hadi Salim(2016)");
MODULE_DESCRIPTION("Inter-FE skb tc_index metadata module");
MODULE_LICENSE("GPL");
-MODULE_ALIAS_IFE_META(IFE_META_SKBTCINDEX);
+MODULE_ALIAS_IFE_META("tcindex");
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/2] net sched actions: fix module auto-loading
From: Roman Mashak @ 2017-10-11 14:50 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, Roman Mashak
In-Reply-To: <1507733430-17860-1-git-send-email-mrv@mojatatu.com>
Macro __stringify_1() can stringify a macro argument, however IFE_META_*
are enums, so they never expand, however request_module expects an integer
in IFE module name, so as a result it always fails to auto-load.
Fixes: ef6980b6becb ("introduce IFE action")
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/act_ife.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 791aeee..e0bc228 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -248,6 +248,20 @@ static int ife_validate_metatype(struct tcf_meta_ops *ops, void *val, int len)
return ret;
}
+static const char *ife_meta_id2name(u32 metaid)
+{
+ switch (metaid) {
+ case IFE_META_SKBMARK:
+ return "skbmark";
+ case IFE_META_PRIO:
+ return "skbprio";
+ case IFE_META_TCINDEX:
+ return "tcindex";
+ default:
+ return "unknown";
+ }
+}
+
/* called when adding new meta information
* under ife->tcf_lock for existing action
*/
@@ -263,7 +277,7 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
if (exists)
spin_unlock_bh(&ife->tcf_lock);
rtnl_unlock();
- request_module("ife-meta-%u", metaid);
+ request_module("ife-meta-%s", ife_meta_id2name(metaid));
rtnl_lock();
if (exists)
spin_lock_bh(&ife->tcf_lock);
--
1.9.1
^ permalink raw reply related
* Skiers List
From: Lorraine Jackson @ 2017-10-11 14:17 UTC (permalink / raw)
To: netdev
Hi,
Greeting of the day!
I hope this note finds you well!
Would you be interested in acquiring an email list of "Skiers" from USA?
Our Databases:- => Fishing Enthusiasts List => Outdoor Enthusiasts List
=> Cruise Travelers List => Power Boat Owners List
=> Camping Enthusiasts List => Sail Boat Owners List
=> Hiking Enthusiasts List => Timeshare Owners List
=> Cycling Enthusiasts List => Travelers List
=> Spa and Resort Visitors List and more.
Each record in the list contains Contact Name (First, Middle and Last Name), Mailing Address, List type and Opt-in email address.
Let me know if you'd be interested in hearing more about it.
Waiting for your valuable and sincere reply.
Best Regards,
Lorraine Jackson
Business Development & Data Specialist
^ 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