* Re: [PATCH RFC 0/2] Add support for warnings to extack
From: Marcelo Ricardo Leitner @ 2018-03-18 18:20 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <5f894da5-6e45-c70c-e061-e7572a4390f1@gmail.com>
On Sun, Mar 18, 2018 at 10:11:52AM -0600, David Ahern wrote:
> On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> > Currently we have the limitation that warnings cannot be reported though
> > extack. For example, when tc flower failed to get offloaded but got
> > installed on software datapath. The hardware failure is not fatal and
> > thus extack is not even shared with the driver, so the error is simply
> > omitted from any logging.
>
> If this set ends up moving forward, the above statement needs to be
> corrected: extack allows non-error messages to be sent back to the user,
> so the above must be talking about some other limitation local to tc.
Right.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message
From: Marcelo Ricardo Leitner @ 2018-03-18 18:19 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <ef243654-c428-b645-7264-6134909744cf@gmail.com>
On Sun, Mar 18, 2018 at 10:11:20AM -0600, David Ahern wrote:
> On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> > Currently extack can carry only a single message, which is usually the
> > error message.
> >
> > This imposes a limitation on a more verbose error reporting. For
> > example, it's not able to carry warning messages together with the error
> > message, or 2 warning messages.
>
>
> The only means for userspace to separate an error message from info or
> warnings is the error in nlmsgerr. If it is non-0, any extack message is
> considered an error else it is a warning.
I don't see your point here.
The proposed patch extends what you said to:
- include warnings on error reports
- allow more than 1 message
With the proposed patch, if nlmsgerr is 0 all messages are considered
as warnings. If it's non-zero, some may be marked as warnings.
AFAICT it is not far from what you described, and still honouring the
main knob, mlmsgerr.
>
>
> >
> > One use case is when dealing with tc offloading. If it failed to
> > offload, and also failed to install on software, it will report only
> > regarding the error about the software datapath, but the error from the
> > hardware path would also have been welcomed.
> >
> > This patch extends extack so it now can carry up to 8 messages and these
> > messages may be prefixed similarly to printk/pr_warning, so thus they
> > can be tagged either was warning or error.
> >
> > Fixed number of messages because supporting a dynamic limit seem to be
> > an overkill for the moment. Remember that this is not meant to be a
> > trace tool, but an error reporting one.
> >
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> > include/linux/netlink.h | 50 +++++++++++++++++++++++++++++-------------------
> > net/netlink/af_netlink.c | 12 +++++++-----
> > 2 files changed, 37 insertions(+), 25 deletions(-)
> >
> > diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> > index f3075d6c7e8229c999ab650537f1e3b11e1f457b..d9780836cf263d4c436d732e9b7a8cde0739ac23 100644
> > --- a/include/linux/netlink.h
> > +++ b/include/linux/netlink.h
> > @@ -71,43 +71,53 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg)
> > * @cookie: cookie data to return to userspace (for success)
> > * @cookie_len: actual cookie data length
> > */
> > +#define NETLINK_MAX_EXTACK_MSGS 8
>
> 8 is way too many. If some change fails because of an error, why would a
Ok. I'm fine with 4 for now.
> single error message not be enough? If it is a not an error, why is more
> than 1 warning message not enough? (I forget the details of the tc
> 'skip_sw' use case)
Because 1 message assumes you have a simple and linear code path being
executed and that it aborts on the first error it encounters.
You could, for example, report several bad arguments at once instead
of having the user to fix & retry until he gets it all right.
You can also have situations like:
Warning: option foo is useless with option bar specified.
(as in: the rule may not work as you intended)
Error: failed to allocate a new node.
The goal is to be able to deliver more information to the
user/software using netlink and be able to log it, for later analysis.
If you have a look at mlx5/core/en_tc.c, there are several
printk(KERN_WARNING or pr_warn calls that should be getting returned
via extack and not to kernel dmesg. That's just one domain and it is
not aware if there is already a message stored in extack or if there
will be another one later.
>
>
> > struct netlink_ext_ack {
> > - const char *_msg;
> > + const char *_msg[NETLINK_MAX_EXTACK_MSGS];
> > const struct nlattr *bad_attr;
> > u8 cookie[NETLINK_MAX_COOKIE_LEN];
> > u8 cookie_len;
> > + u8 _msg_count;
> > };
> >
> > -/* Always use this macro, this allows later putting the
> > - * message into a separate section or such for things
> > - * like translation or listing all possible messages.
> > - * Currently string formatting is not supported (due
> > - * to the lack of an output buffer.)
> > +/* Always use these macros, this allows later putting
> > + * the message into a separate section or such for
> > + * things like translation or listing all possible
> > + * messages. Currently string formatting is not
> > + * supported (due to the lack of an output buffer.)
> > */
> > -#define NL_SET_ERR_MSG(extack, msg) do { \
> > - static const char __msg[] = msg; \
> > - struct netlink_ext_ack *__extack = (extack); \
> > - \
> > - if (__extack) \
> > - __extack->_msg = __msg; \
> > +#define NL_SET_MSG(extack, msg) do { \
> > + static const char __msg[] = msg; \
> > + struct netlink_ext_ack *__extack = (extack); \
> > + \
> > + if (__extack && \
> > + !WARN_ON(__extack->_msg_count >= NETLINK_MAX_EXTACK_MSGS)) \
> > + __extack->_msg[__extack->_msg_count++] = __msg; \
> > } while (0)
> >
> > +#define NL_SET_ERR_MSG(extack, msg) NL_SET_MSG(extack, msg)
> > +#define NL_SET_WARN_MSG(extack, msg) NL_SET_MSG(extack, KERN_WARNING msg)
> > +
> > #define NL_SET_ERR_MSG_MOD(extack, msg) \
> > NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg)
> > +#define NL_SET_WARN_MSG_MOD(extack, msg) \
> > + NL_SET_WARN_MSG((extack), KBUILD_MODNAME ": " msg)
> > +
>
> Adding separate macros for error versus warning is confusing since from
> an extack perspective a message is a message and there is no uapi to
> separate them.
Are you saying the markings at beginning of the messages are not
possible? If that's the case, we probably can think of something else,
as I see value in being able to deliver warnings together with errors.
M.
^ permalink raw reply
* Re: [PATCH net-next 00/10 v2] selftests: pmtu: Add further vti/vti6 MTU and PMTU tests
From: Stefano Brivio @ 2018-03-18 17:42 UTC (permalink / raw)
To: David Ahern; +Cc: David S . Miller, Sabrina Dubroca, Steffen Klassert, netdev
In-Reply-To: <42e5ee79-1cca-92f8-4b0f-8cdbfd09e071@gmail.com>
On Sun, 18 Mar 2018 11:31:10 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 3/16/18 7:31 PM, Stefano Brivio wrote:
> > Patches 5/10 to 10/10 add tests to verify default MTU assignment
> > for vti4 and vti6 interfaces, to check that MTU values set on new
> > link and link changes are properly taken and validated, and to
> > verify PMTU exceptions on vti4 interfaces.
> >
> > Patch 1/10 reverses function return codes as suggested by David
> > Ahern.
> >
> > Patch 2/10 fixes the helper to fetch exceptions MTU to run in the
> > passed namespace.
> >
> > Patches 3/10 and 4/10 are preparation work to make it easier to
> > introduce those tests.
> >
> > v2: Reverse return codes, and make output prettier in 4/9 by
> > using padded printf, test descriptions and buffered error
> > strings. Remove accidental output to /dev/kmsg from 10/10
> > (was 9/9).
>
> pulling pmtu.sh from net-next I see log messages on console:
>
> [ 890.741117] ip netns exec ns-kFUQx6 ip link set vti6_a type vti6
> remote fc00:1001::0 local fc00:1001::0
> [ 890.837569] ip netns exec ns-kFUQx6 ip link set vti6_a mtu 1280 type
> vti6 remote fc00:1000::0 local fc00:1000::0
Thanks for checking. I accidentally left two more prints to kernel log
in 10/10, I'll send another patch to remove them.
--
Stefano
^ permalink raw reply
* Re: [PATCH RFC 0/2] Add support for warnings to extack
From: Marcelo Ricardo Leitner @ 2018-03-18 17:38 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, Alexander Aring, Jiri Pirko, David Ahern
In-Reply-To: <20180316150518.0641d03a@cakuba.netronome.com>
On Fri, Mar 16, 2018 at 03:05:18PM -0700, Jakub Kicinski wrote:
> CC: David Ahern <dsahern@gmail.com>
>
> On Fri, 16 Mar 2018 16:23:08 -0300, Marcelo Ricardo Leitner wrote:
> > Currently we have the limitation that warnings cannot be reported though
> > extack. For example, when tc flower failed to get offloaded but got
> > installed on software datapath. The hardware failure is not fatal and
> > thus extack is not even shared with the driver, so the error is simply
> > omitted from any logging.
> >
> > The idea here is to allow such kind of warnings to get through and be
> > available for the sysadmin or the tool managing such commands (like Open
> > vSwitch), so that if this happens, we will have such log message in a
> > file later.
> >
> > The first patch extends extack to support more than one message and with
> > different log level (currently only error and warning). The second
> > shares extack with the drivers regardless of skip_sw.
> >
> > The iproute patch also follows.
> >
> > This kernel change is backward compatible with older iproute because
> > iproute will only process the last message, which should be the error
> > one in case of failure, or a warning if it suceeded.
> >
> > The iproute change is compatible with older kernels because it will find
> > only one message to be processed and will handle it properly.
> >
> > With this patches, this is now possible:
> > # tc qdisc add dev p7p1 ingress
> > # tc filter add dev p7p1 parent ffff: protocol ip prio 1 flower \
> > src_mac ec:13:db:00:00:00 dst_mac ec:14:c2:00:00:00 \
> > src_ip 56.0.0.0 dst_ip 55.0.0.0 action drop
> > Warning: TC offload is disabled on net device.
> > # echo $?
> > 0
>
> IMHO this set does more and less than is required to solve the
> problem.
>
> The way I understand it is we don't want HW offload errors/warnings to
> be printed to unsuspecting users who didn't specify any skip_* flags.
> What carries the message and whether it's explicitly marked as warning
> or error does not change the fact that user of the SW fwd path may not
> want to not be bothered by offload warnings.
Fair enough. We can then have a 'tc -v' option to enable this more
verbose logging.
>
> There maybe well be value in ability to report multiple messages. But
> for opt-in warning messages I would be leaning towards:
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index e828d31be5dae0ae8c69016dfde50379296484aa..7cec393bb47974b48a6d510b8aa84534a7a98594 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -705,8 +705,7 @@ tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common,
> cls_common->chain_index = tp->chain->index;
> cls_common->protocol = tp->protocol;
> cls_common->prio = tp->prio;
> - if (tc_skip_sw(flags))
> + if (tc_skip_sw(flags) || flags & TCA_CLS_FLAGS_OFFLOAD_VERBOSE)
> cls_common->extack = extack;
> }
>
> enum tc_fl_command {
>
> That is admittedly quite conservative. Esp. in case of flower, cls_bpf
> is used in SW far more than HW, not to mention qdisc offload (although
> flag would be different there)!
Yeah, or something more generic, as a general -v / --verbose option.
M.
^ permalink raw reply
* Re: [PATCH net-next 00/10 v2] selftests: pmtu: Add further vti/vti6 MTU and PMTU tests
From: David Ahern @ 2018-03-18 17:31 UTC (permalink / raw)
To: Stefano Brivio, David S . Miller
Cc: Sabrina Dubroca, Steffen Klassert, netdev
In-Reply-To: <cover.1521249420.git.sbrivio@redhat.com>
On 3/16/18 7:31 PM, Stefano Brivio wrote:
> Patches 5/10 to 10/10 add tests to verify default MTU assignment
> for vti4 and vti6 interfaces, to check that MTU values set on new
> link and link changes are properly taken and validated, and to
> verify PMTU exceptions on vti4 interfaces.
>
> Patch 1/10 reverses function return codes as suggested by David
> Ahern.
>
> Patch 2/10 fixes the helper to fetch exceptions MTU to run in the
> passed namespace.
>
> Patches 3/10 and 4/10 are preparation work to make it easier to
> introduce those tests.
>
> v2: Reverse return codes, and make output prettier in 4/9 by
> using padded printf, test descriptions and buffered error
> strings. Remove accidental output to /dev/kmsg from 10/10
> (was 9/9).
pulling pmtu.sh from net-next I see log messages on console:
[ 890.741117] ip netns exec ns-kFUQx6 ip link set vti6_a type vti6
remote fc00:1001::0 local fc00:1001::0
[ 890.837569] ip netns exec ns-kFUQx6 ip link set vti6_a mtu 1280 type
vti6 remote fc00:1000::0 local fc00:1000::0
^ permalink raw reply
* Re: [PATCH iproute2] treat "default" and "all"/"any" addresses differenty
From: Alexander Zubkov @ 2018-03-18 17:02 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Luca Boccassi, netdev@vger.kernel.org, Serhey Popovych
In-Reply-To: <1521391825-28636-1-git-send-email-green@msu.ru>
This version of patch is for master now.
18.03.2018, 17:50, "Alexander Zubkov" <green@msu.ru>:
> Debian maintainer found that basic command:
> # ip route flush all
> No longer worked as expected which breaks user scripts and
> expectations. It no longer flushed all IPv4 routes.
>
> Recently behavior of "default" prefix parameter was corrected. But at
> the same time behavior of "all"/"any" was altered too, because they
> were the same branch of the code. As those parameters mean different,
> they need to be treated differently in code too. This patch reflects
> the difference.
>
> Also after mentioned change, address parsing code was changed more
> and address family was set explicitly even for "all"/"any" addresses.
> And that broke matching conditions further. This patch fixes that too
> and returns AF_UNSPEC to "all"/"any" address.
>
> Now "default" is treated as top-level prefix (for example 0.0.0.0/0 in
> IPv4) and "all"/"any" always matches anything in exact, root and match
> modes.
>
> Reported-by: Luca Boccassi <bluca@debian.org>
> Signed-off-by: Alexander Zubkov <green@msu.ru>
> ---
> lib/utils.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/lib/utils.c b/lib/utils.c
> index 379739d..eba4fa7 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -560,14 +560,23 @@ static int __get_addr_1(inet_prefix *addr, const char *name, int family)
> {
> memset(addr, 0, sizeof(*addr));
>
> - if (strcmp(name, "default") == 0 ||
> - strcmp(name, "all") == 0 ||
> - strcmp(name, "any") == 0) {
> + if (strcmp(name, "default") == 0) {
> if ((family == AF_DECnet) || (family == AF_MPLS))
> return -1;
> addr->family = (family != AF_UNSPEC) ? family : AF_INET;
> addr->bytelen = af_byte_len(addr->family);
> addr->bitlen = -2;
> + addr->flags |= PREFIXLEN_SPECIFIED;
> + return 0;
> + }
> +
> + if (strcmp(name, "all") == 0 ||
> + strcmp(name, "any") == 0) {
> + if ((family == AF_DECnet) || (family == AF_MPLS))
> + return -1;
> + addr->family = AF_UNSPEC;
> + addr->bytelen = 0;
> + addr->bitlen = -2;
> return 0;
> }
>
> @@ -695,7 +704,7 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family)
>
> bitlen = af_bit_len(dst->family);
>
> - flags = PREFIXLEN_SPECIFIED;
> + flags = 0;
> if (slash) {
> unsigned int plen;
>
> @@ -706,12 +715,11 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family)
> if (plen > bitlen)
> return -1;
>
> + flags |= PREFIXLEN_SPECIFIED;
> bitlen = plen;
> } else {
> if (dst->bitlen == -2)
> bitlen = 0;
> - else
> - flags = 0;
> }
>
> dst->flags |= flags;
> --
> 1.9.1
^ permalink raw reply
* [PATCH iproute2] treat "default" and "all"/"any" addresses differenty
From: Alexander Zubkov @ 2018-03-18 16:50 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Luca Boccassi, netdev, Serhey Popovych, Alexander Zubkov
In-Reply-To: <2ee0179a-79c8-8504-afe1-6310ed4e858b@msu.ru>
Debian maintainer found that basic command:
# ip route flush all
No longer worked as expected which breaks user scripts and
expectations. It no longer flushed all IPv4 routes.
Recently behavior of "default" prefix parameter was corrected. But at
the same time behavior of "all"/"any" was altered too, because they
were the same branch of the code. As those parameters mean different,
they need to be treated differently in code too. This patch reflects
the difference.
Also after mentioned change, address parsing code was changed more
and address family was set explicitly even for "all"/"any" addresses.
And that broke matching conditions further. This patch fixes that too
and returns AF_UNSPEC to "all"/"any" address.
Now "default" is treated as top-level prefix (for example 0.0.0.0/0 in
IPv4) and "all"/"any" always matches anything in exact, root and match
modes.
Reported-by: Luca Boccassi <bluca@debian.org>
Signed-off-by: Alexander Zubkov <green@msu.ru>
---
lib/utils.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/lib/utils.c b/lib/utils.c
index 379739d..eba4fa7 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -560,14 +560,23 @@ static int __get_addr_1(inet_prefix *addr, const char *name, int family)
{
memset(addr, 0, sizeof(*addr));
- if (strcmp(name, "default") == 0 ||
- strcmp(name, "all") == 0 ||
- strcmp(name, "any") == 0) {
+ if (strcmp(name, "default") == 0) {
if ((family == AF_DECnet) || (family == AF_MPLS))
return -1;
addr->family = (family != AF_UNSPEC) ? family : AF_INET;
addr->bytelen = af_byte_len(addr->family);
addr->bitlen = -2;
+ addr->flags |= PREFIXLEN_SPECIFIED;
+ return 0;
+ }
+
+ if (strcmp(name, "all") == 0 ||
+ strcmp(name, "any") == 0) {
+ if ((family == AF_DECnet) || (family == AF_MPLS))
+ return -1;
+ addr->family = AF_UNSPEC;
+ addr->bytelen = 0;
+ addr->bitlen = -2;
return 0;
}
@@ -695,7 +704,7 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family)
bitlen = af_bit_len(dst->family);
- flags = PREFIXLEN_SPECIFIED;
+ flags = 0;
if (slash) {
unsigned int plen;
@@ -706,12 +715,11 @@ int get_prefix_1(inet_prefix *dst, char *arg, int family)
if (plen > bitlen)
return -1;
+ flags |= PREFIXLEN_SPECIFIED;
bitlen = plen;
} else {
if (dst->bitlen == -2)
bitlen = 0;
- else
- flags = 0;
}
dst->flags |= flags;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH RFC 0/2] Add support for warnings to extack
From: David Ahern @ 2018-03-18 16:11 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, netdev
Cc: Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <cover.1521226621.git.marcelo.leitner@gmail.com>
On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> Currently we have the limitation that warnings cannot be reported though
> extack. For example, when tc flower failed to get offloaded but got
> installed on software datapath. The hardware failure is not fatal and
> thus extack is not even shared with the driver, so the error is simply
> omitted from any logging.
If this set ends up moving forward, the above statement needs to be
corrected: extack allows non-error messages to be sent back to the user,
so the above must be talking about some other limitation local to tc.
^ permalink raw reply
* Re: [PATCH RFC 1/2] netlink: extend extack so it can carry more than one message
From: David Ahern @ 2018-03-18 16:11 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, netdev
Cc: Alexander Aring, Jiri Pirko, Jakub Kicinski
In-Reply-To: <673abaddb26351826ca454f46d1271f1f4814c56.1521226621.git.marcelo.leitner@gmail.com>
On 3/16/18 1:23 PM, Marcelo Ricardo Leitner wrote:
> Currently extack can carry only a single message, which is usually the
> error message.
>
> This imposes a limitation on a more verbose error reporting. For
> example, it's not able to carry warning messages together with the error
> message, or 2 warning messages.
The only means for userspace to separate an error message from info or
warnings is the error in nlmsgerr. If it is non-0, any extack message is
considered an error else it is a warning.
>
> One use case is when dealing with tc offloading. If it failed to
> offload, and also failed to install on software, it will report only
> regarding the error about the software datapath, but the error from the
> hardware path would also have been welcomed.
>
> This patch extends extack so it now can carry up to 8 messages and these
> messages may be prefixed similarly to printk/pr_warning, so thus they
> can be tagged either was warning or error.
>
> Fixed number of messages because supporting a dynamic limit seem to be
> an overkill for the moment. Remember that this is not meant to be a
> trace tool, but an error reporting one.
>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/linux/netlink.h | 50 +++++++++++++++++++++++++++++-------------------
> net/netlink/af_netlink.c | 12 +++++++-----
> 2 files changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index f3075d6c7e8229c999ab650537f1e3b11e1f457b..d9780836cf263d4c436d732e9b7a8cde0739ac23 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -71,43 +71,53 @@ netlink_kernel_create(struct net *net, int unit, struct netlink_kernel_cfg *cfg)
> * @cookie: cookie data to return to userspace (for success)
> * @cookie_len: actual cookie data length
> */
> +#define NETLINK_MAX_EXTACK_MSGS 8
8 is way too many. If some change fails because of an error, why would a
single error message not be enough? If it is a not an error, why is more
than 1 warning message not enough? (I forget the details of the tc
'skip_sw' use case)
> struct netlink_ext_ack {
> - const char *_msg;
> + const char *_msg[NETLINK_MAX_EXTACK_MSGS];
> const struct nlattr *bad_attr;
> u8 cookie[NETLINK_MAX_COOKIE_LEN];
> u8 cookie_len;
> + u8 _msg_count;
> };
>
> -/* Always use this macro, this allows later putting the
> - * message into a separate section or such for things
> - * like translation or listing all possible messages.
> - * Currently string formatting is not supported (due
> - * to the lack of an output buffer.)
> +/* Always use these macros, this allows later putting
> + * the message into a separate section or such for
> + * things like translation or listing all possible
> + * messages. Currently string formatting is not
> + * supported (due to the lack of an output buffer.)
> */
> -#define NL_SET_ERR_MSG(extack, msg) do { \
> - static const char __msg[] = msg; \
> - struct netlink_ext_ack *__extack = (extack); \
> - \
> - if (__extack) \
> - __extack->_msg = __msg; \
> +#define NL_SET_MSG(extack, msg) do { \
> + static const char __msg[] = msg; \
> + struct netlink_ext_ack *__extack = (extack); \
> + \
> + if (__extack && \
> + !WARN_ON(__extack->_msg_count >= NETLINK_MAX_EXTACK_MSGS)) \
> + __extack->_msg[__extack->_msg_count++] = __msg; \
> } while (0)
>
> +#define NL_SET_ERR_MSG(extack, msg) NL_SET_MSG(extack, msg)
> +#define NL_SET_WARN_MSG(extack, msg) NL_SET_MSG(extack, KERN_WARNING msg)
> +
> #define NL_SET_ERR_MSG_MOD(extack, msg) \
> NL_SET_ERR_MSG((extack), KBUILD_MODNAME ": " msg)
> +#define NL_SET_WARN_MSG_MOD(extack, msg) \
> + NL_SET_WARN_MSG((extack), KBUILD_MODNAME ": " msg)
> +
Adding separate macros for error versus warning is confusing since from
an extack perspective a message is a message and there is no uapi to
separate them.
^ permalink raw reply
* Re: [PATCH v11 crypto 01/12] tls: support for Inline tls record (fwd)
From: Julia Lawall @ 2018-03-18 16:01 UTC (permalink / raw)
To: Atul Gupta
Cc: davejwatson, davem, herbert, sd, sbrivio, linux-crypto, netdev,
ganeshgr, sd, sbrivio, linux-crypto, netdev, ganeshgr, kbuild-all
ctx is dereferenced on line 258 but has been freed on line 229.
julia
---------- Forwarded message ----------
Date: Sun, 18 Mar 2018 18:05:25 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v11 crypto 01/12] tls: support for Inline tls record
CC: kbuild-all@01.org
In-Reply-To: <1521214661-28928-1-git-send-email-atul.gupta@chelsio.com>
References: <1521214661-28928-1-git-send-email-atul.gupta@chelsio.com>
TO: Atul Gupta <atul.gupta@chelsio.com>
CC: davejwatson@fb.com, davem@davemloft.net, herbert@gondor.apana.org.au, sd@queasysnail.net, sbrivio@redhat.com, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, ganeshgr@chelsio.com
CC: sd@queasysnail.net, sbrivio@redhat.com, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, ganeshgr@chelsio.com
Hi Atul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.16-rc4]
[cannot apply to next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Atul-Gupta/tls-support-for-Inline-tls-record/20180318-162840
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
>> net/tls/tls_main.c:258:5-8: ERROR: reference preceded by free on line 229
# https://github.com/0day-ci/linux/commit/be47378786b9d9874dfc3ab57504565275c7b3ff
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout be47378786b9d9874dfc3ab57504565275c7b3ff
vim +258 net/tls/tls_main.c
3c4d75591 Dave Watson 2017-06-14 218
3c4d75591 Dave Watson 2017-06-14 219 static void tls_sk_proto_close(struct sock *sk, long timeout)
3c4d75591 Dave Watson 2017-06-14 220 {
3c4d75591 Dave Watson 2017-06-14 221 struct tls_context *ctx = tls_get_ctx(sk);
3c4d75591 Dave Watson 2017-06-14 222 long timeo = sock_sndtimeo(sk, 0);
3c4d75591 Dave Watson 2017-06-14 223 void (*sk_proto_close)(struct sock *sk, long timeout);
3c4d75591 Dave Watson 2017-06-14 224
3c4d75591 Dave Watson 2017-06-14 225 lock_sock(sk);
ff45d820a Ilya Lesokhin 2017-11-13 226 sk_proto_close = ctx->sk_proto_close;
ff45d820a Ilya Lesokhin 2017-11-13 227
ff45d820a Ilya Lesokhin 2017-11-13 228 if (ctx->tx_conf == TLS_BASE_TX) {
ff45d820a Ilya Lesokhin 2017-11-13 @229 kfree(ctx);
ff45d820a Ilya Lesokhin 2017-11-13 230 goto skip_tx_cleanup;
ff45d820a Ilya Lesokhin 2017-11-13 231 }
3c4d75591 Dave Watson 2017-06-14 232
3c4d75591 Dave Watson 2017-06-14 233 if (!tls_complete_pending_work(sk, ctx, 0, &timeo))
3c4d75591 Dave Watson 2017-06-14 234 tls_handle_open_record(sk, 0);
3c4d75591 Dave Watson 2017-06-14 235
3c4d75591 Dave Watson 2017-06-14 236 if (ctx->partially_sent_record) {
3c4d75591 Dave Watson 2017-06-14 237 struct scatterlist *sg = ctx->partially_sent_record;
3c4d75591 Dave Watson 2017-06-14 238
3c4d75591 Dave Watson 2017-06-14 239 while (1) {
3c4d75591 Dave Watson 2017-06-14 240 put_page(sg_page(sg));
3c4d75591 Dave Watson 2017-06-14 241 sk_mem_uncharge(sk, sg->length);
3c4d75591 Dave Watson 2017-06-14 242
3c4d75591 Dave Watson 2017-06-14 243 if (sg_is_last(sg))
3c4d75591 Dave Watson 2017-06-14 244 break;
3c4d75591 Dave Watson 2017-06-14 245 sg++;
3c4d75591 Dave Watson 2017-06-14 246 }
3c4d75591 Dave Watson 2017-06-14 247 }
ff45d820a Ilya Lesokhin 2017-11-13 248
3c4d75591 Dave Watson 2017-06-14 249 kfree(ctx->rec_seq);
3c4d75591 Dave Watson 2017-06-14 250 kfree(ctx->iv);
3c4d75591 Dave Watson 2017-06-14 251
ff45d820a Ilya Lesokhin 2017-11-13 252 if (ctx->tx_conf == TLS_SW_TX)
ff45d820a Ilya Lesokhin 2017-11-13 253 tls_sw_free_tx_resources(sk);
3c4d75591 Dave Watson 2017-06-14 254
ff45d820a Ilya Lesokhin 2017-11-13 255 skip_tx_cleanup:
3c4d75591 Dave Watson 2017-06-14 256 release_sock(sk);
3c4d75591 Dave Watson 2017-06-14 257 sk_proto_close(sk, timeout);
be4737878 Atul Gupta 2018-03-16 @258 if (ctx->tx_conf == TLS_HW_RECORD)
be4737878 Atul Gupta 2018-03-16 259 kfree(ctx);
3c4d75591 Dave Watson 2017-06-14 260 }
3c4d75591 Dave Watson 2017-06-14 261
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH v11 crypto 12/12] crypto: chtls - Makefile Kconfig (fwd)
From: Julia Lawall @ 2018-03-18 15:59 UTC (permalink / raw)
To: Atul Gupta
Cc: davejwatson, davem, herbert, sd, sbrivio, linux-crypto, netdev,
ganeshgr, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11129 bytes --]
Please check the indentation on line 1655.
thanks,
julia
---------- Forwarded message ----------
Date: Sun, 18 Mar 2018 18:15:36 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v11 crypto 12/12] crypto: chtls - Makefile Kconfig
CC: kbuild-all@01.org
In-Reply-To: <1521214661-28928-12-git-send-email-atul.gupta@chelsio.com>
References: <1521214661-28928-12-git-send-email-atul.gupta@chelsio.com>
TO: Atul Gupta <atul.gupta@chelsio.com>
CC: davejwatson@fb.com, davem@davemloft.net, herbert@gondor.apana.org.au
CC: sd@queasysnail.net, sbrivio@redhat.com, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, ganeshgr@chelsio.com
Hi Atul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[cannot apply to next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Atul-Gupta/tls-support-for-Inline-tls-record/20180318-162840
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago
All error/warnings (new ones prefixed by >>):
drivers/crypto/chelsio/chtls/chtls_io.c: In function 'chtls_expansion_size':
>> drivers/crypto/chelsio/chtls/chtls_io.c:457:2: error: expected ',' or ';' before 'int'
int expnsize, frcn, fraglast, fragsize;
^~~
>> drivers/crypto/chelsio/chtls/chtls_io.c:461:3: error: 'fragsize' undeclared (first use in this function); did you mean 'ivs_size'?
fragsize = hws->mfs;
^~~~~~~~
ivs_size
drivers/crypto/chelsio/chtls/chtls_io.c:461:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/crypto/chelsio/chtls/chtls_io.c:465:4: error: 'frcnt' undeclared (first use in this function); did you mean 'pducnt'?
frcnt = (data_len / fragsize);
^~~~~
pducnt
>> drivers/crypto/chelsio/chtls/chtls_io.c:468:4: error: 'expnsize' undeclared (first use in this function); did you mean 'fragsize'?
expnsize = frcnt * expppdu;
^~~~~~~~
fragsize
>> drivers/crypto/chelsio/chtls/chtls_io.c:480:4: error: 'fraglast' undeclared (first use in this function); did you mean 'rb_last'?
fraglast = data_len % fragsize;
^~~~~~~~
rb_last
drivers/crypto/chelsio/chtls/chtls_io.c: In function 'peekmsg':
>> drivers/crypto/chelsio/chtls/chtls_io.c:1653:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!copied)
^~
drivers/crypto/chelsio/chtls/chtls_io.c:1655:6: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
break;
^~~~~
drivers/crypto/chelsio/chtls/chtls_io.c: In function 'chtls_expansion_size':
>> drivers/crypto/chelsio/chtls/chtls_io.c:492:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
coccinelle warnings: (new ones prefixed by >>)
>> drivers/crypto/chelsio/chtls/chtls_io.c:1654:5-22: code aligned with following code on line 1655
# https://github.com/0day-ci/linux/commit/635907fe348f84b525d7ce16ae8f2a9b82c631e3
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 635907fe348f84b525d7ce16ae8f2a9b82c631e3
vim +1654 drivers/crypto/chelsio/chtls/chtls_io.c
8ae18d74 Atul Gupta 2018-03-16 1542
8ae18d74 Atul Gupta 2018-03-16 1543 /*
8ae18d74 Atul Gupta 2018-03-16 1544 * Peek at data in a socket's receive buffer.
8ae18d74 Atul Gupta 2018-03-16 1545 */
8ae18d74 Atul Gupta 2018-03-16 1546 static int peekmsg(struct sock *sk, struct msghdr *msg,
8ae18d74 Atul Gupta 2018-03-16 1547 size_t len, int nonblock, int flags)
8ae18d74 Atul Gupta 2018-03-16 1548 {
8ae18d74 Atul Gupta 2018-03-16 1549 struct tcp_sock *tp = tcp_sk(sk);
8ae18d74 Atul Gupta 2018-03-16 1550 struct sk_buff *skb;
8ae18d74 Atul Gupta 2018-03-16 1551 u32 peek_seq, offset;
8ae18d74 Atul Gupta 2018-03-16 1552 int copied = 0;
8ae18d74 Atul Gupta 2018-03-16 1553 size_t avail; /* amount of available data in current skb */
8ae18d74 Atul Gupta 2018-03-16 1554 long timeo;
8ae18d74 Atul Gupta 2018-03-16 1555
8ae18d74 Atul Gupta 2018-03-16 1556 lock_sock(sk);
8ae18d74 Atul Gupta 2018-03-16 1557 timeo = sock_rcvtimeo(sk, nonblock);
8ae18d74 Atul Gupta 2018-03-16 1558 peek_seq = tp->copied_seq;
8ae18d74 Atul Gupta 2018-03-16 1559
8ae18d74 Atul Gupta 2018-03-16 1560 do {
8ae18d74 Atul Gupta 2018-03-16 1561 if (unlikely(tp->urg_data && tp->urg_seq == peek_seq)) {
8ae18d74 Atul Gupta 2018-03-16 1562 if (copied)
8ae18d74 Atul Gupta 2018-03-16 1563 break;
8ae18d74 Atul Gupta 2018-03-16 1564 if (signal_pending(current)) {
8ae18d74 Atul Gupta 2018-03-16 1565 copied = timeo ? sock_intr_errno(timeo) :
8ae18d74 Atul Gupta 2018-03-16 1566 -EAGAIN;
8ae18d74 Atul Gupta 2018-03-16 1567 break;
8ae18d74 Atul Gupta 2018-03-16 1568 }
8ae18d74 Atul Gupta 2018-03-16 1569 }
8ae18d74 Atul Gupta 2018-03-16 1570
8ae18d74 Atul Gupta 2018-03-16 1571 skb_queue_walk(&sk->sk_receive_queue, skb) {
8ae18d74 Atul Gupta 2018-03-16 1572 offset = peek_seq - ULP_SKB_CB(skb)->seq;
8ae18d74 Atul Gupta 2018-03-16 1573 if (offset < skb->len)
8ae18d74 Atul Gupta 2018-03-16 1574 goto found_ok_skb;
8ae18d74 Atul Gupta 2018-03-16 1575 }
8ae18d74 Atul Gupta 2018-03-16 1576
8ae18d74 Atul Gupta 2018-03-16 1577 /* empty receive queue */
8ae18d74 Atul Gupta 2018-03-16 1578 if (copied)
8ae18d74 Atul Gupta 2018-03-16 1579 break;
8ae18d74 Atul Gupta 2018-03-16 1580 if (sock_flag(sk, SOCK_DONE))
8ae18d74 Atul Gupta 2018-03-16 1581 break;
8ae18d74 Atul Gupta 2018-03-16 1582 if (sk->sk_err) {
8ae18d74 Atul Gupta 2018-03-16 1583 copied = sock_error(sk);
8ae18d74 Atul Gupta 2018-03-16 1584 break;
8ae18d74 Atul Gupta 2018-03-16 1585 }
8ae18d74 Atul Gupta 2018-03-16 1586 if (sk_no_receive(sk))
8ae18d74 Atul Gupta 2018-03-16 1587 break;
8ae18d74 Atul Gupta 2018-03-16 1588 if (sk->sk_state == TCP_CLOSE) {
8ae18d74 Atul Gupta 2018-03-16 1589 copied = -ENOTCONN;
8ae18d74 Atul Gupta 2018-03-16 1590 break;
8ae18d74 Atul Gupta 2018-03-16 1591 }
8ae18d74 Atul Gupta 2018-03-16 1592 if (!timeo) {
8ae18d74 Atul Gupta 2018-03-16 1593 copied = -EAGAIN;
8ae18d74 Atul Gupta 2018-03-16 1594 break;
8ae18d74 Atul Gupta 2018-03-16 1595 }
8ae18d74 Atul Gupta 2018-03-16 1596 if (signal_pending(current)) {
8ae18d74 Atul Gupta 2018-03-16 1597 copied = sock_intr_errno(timeo);
8ae18d74 Atul Gupta 2018-03-16 1598 break;
8ae18d74 Atul Gupta 2018-03-16 1599 }
8ae18d74 Atul Gupta 2018-03-16 1600
8ae18d74 Atul Gupta 2018-03-16 1601 if (sk->sk_backlog.tail) {
8ae18d74 Atul Gupta 2018-03-16 1602 /* Do not sleep, just process backlog. */
8ae18d74 Atul Gupta 2018-03-16 1603 release_sock(sk);
8ae18d74 Atul Gupta 2018-03-16 1604 lock_sock(sk);
8ae18d74 Atul Gupta 2018-03-16 1605 } else {
8ae18d74 Atul Gupta 2018-03-16 1606 sk_wait_data(sk, &timeo, NULL);
8ae18d74 Atul Gupta 2018-03-16 1607 }
8ae18d74 Atul Gupta 2018-03-16 1608
8ae18d74 Atul Gupta 2018-03-16 1609 if (unlikely(peek_seq != tp->copied_seq)) {
8ae18d74 Atul Gupta 2018-03-16 1610 if (net_ratelimit())
8ae18d74 Atul Gupta 2018-03-16 1611 pr_info("TCP(%s:%d), race in MSG_PEEK.\n",
8ae18d74 Atul Gupta 2018-03-16 1612 current->comm, current->pid);
8ae18d74 Atul Gupta 2018-03-16 1613 peek_seq = tp->copied_seq;
8ae18d74 Atul Gupta 2018-03-16 1614 }
8ae18d74 Atul Gupta 2018-03-16 1615 continue;
8ae18d74 Atul Gupta 2018-03-16 1616
8ae18d74 Atul Gupta 2018-03-16 1617 found_ok_skb:
8ae18d74 Atul Gupta 2018-03-16 1618 avail = skb->len - offset;
8ae18d74 Atul Gupta 2018-03-16 1619 if (len < avail)
8ae18d74 Atul Gupta 2018-03-16 1620 avail = len;
8ae18d74 Atul Gupta 2018-03-16 1621 /*
8ae18d74 Atul Gupta 2018-03-16 1622 * Do we have urgent data here? We need to skip over the
8ae18d74 Atul Gupta 2018-03-16 1623 * urgent byte.
8ae18d74 Atul Gupta 2018-03-16 1624 */
8ae18d74 Atul Gupta 2018-03-16 1625 if (unlikely(tp->urg_data)) {
8ae18d74 Atul Gupta 2018-03-16 1626 u32 urg_offset = tp->urg_seq - peek_seq;
8ae18d74 Atul Gupta 2018-03-16 1627
8ae18d74 Atul Gupta 2018-03-16 1628 if (urg_offset < avail) {
8ae18d74 Atul Gupta 2018-03-16 1629 /*
8ae18d74 Atul Gupta 2018-03-16 1630 * The amount of data we are preparing to copy
8ae18d74 Atul Gupta 2018-03-16 1631 * contains urgent data.
8ae18d74 Atul Gupta 2018-03-16 1632 */
8ae18d74 Atul Gupta 2018-03-16 1633 if (!urg_offset) { /* First byte is urgent */
8ae18d74 Atul Gupta 2018-03-16 1634 if (!sock_flag(sk, SOCK_URGINLINE)) {
8ae18d74 Atul Gupta 2018-03-16 1635 peek_seq++;
8ae18d74 Atul Gupta 2018-03-16 1636 offset++;
8ae18d74 Atul Gupta 2018-03-16 1637 avail--;
8ae18d74 Atul Gupta 2018-03-16 1638 }
8ae18d74 Atul Gupta 2018-03-16 1639 if (!avail)
8ae18d74 Atul Gupta 2018-03-16 1640 continue;
8ae18d74 Atul Gupta 2018-03-16 1641 } else {
8ae18d74 Atul Gupta 2018-03-16 1642 /* stop short of the urgent data */
8ae18d74 Atul Gupta 2018-03-16 1643 avail = urg_offset;
8ae18d74 Atul Gupta 2018-03-16 1644 }
8ae18d74 Atul Gupta 2018-03-16 1645 }
8ae18d74 Atul Gupta 2018-03-16 1646 }
8ae18d74 Atul Gupta 2018-03-16 1647
8ae18d74 Atul Gupta 2018-03-16 1648 /*
8ae18d74 Atul Gupta 2018-03-16 1649 * If MSG_TRUNC is specified the data is discarded.
8ae18d74 Atul Gupta 2018-03-16 1650 */
8ae18d74 Atul Gupta 2018-03-16 1651 if (likely(!(flags & MSG_TRUNC)))
8ae18d74 Atul Gupta 2018-03-16 1652 if (skb_copy_datagram_msg(skb, offset, msg, len)) {
8ae18d74 Atul Gupta 2018-03-16 @1653 if (!copied)
8ae18d74 Atul Gupta 2018-03-16 @1654 copied = -EFAULT;
8ae18d74 Atul Gupta 2018-03-16 @1655 break;
8ae18d74 Atul Gupta 2018-03-16 1656 }
8ae18d74 Atul Gupta 2018-03-16 1657 peek_seq += avail;
8ae18d74 Atul Gupta 2018-03-16 1658 copied += avail;
8ae18d74 Atul Gupta 2018-03-16 1659 len -= avail;
8ae18d74 Atul Gupta 2018-03-16 1660 } while (len > 0);
8ae18d74 Atul Gupta 2018-03-16 1661
8ae18d74 Atul Gupta 2018-03-16 1662 release_sock(sk);
8ae18d74 Atul Gupta 2018-03-16 1663 return copied;
8ae18d74 Atul Gupta 2018-03-16 1664 }
8ae18d74 Atul Gupta 2018-03-16 1665
:::::: The code at line 1654 was first introduced by commit
:::::: 8ae18d74c01914f49f175dd5375457e6e47d5a0d crypto: chtls - Inline TLS record Rx
:::::: TO: Atul Gupta <atul.gupta@chelsio.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: Type: application/gzip, Size: 63078 bytes --]
^ permalink raw reply
* [PATCH net] devlink: Remove redundant free on error path
From: Arkadi Sharshevsky @ 2018-03-18 15:37 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jiri, Arkadi Sharshevsky
The current code performs unneeded free. Remove the redundant skb freeing
during the error path.
Fixes: 1555d204e743 ("devlink: Support for pipeline debug (dpipe)")
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
---
net/core/devlink.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f23e5ed..7917838 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1798,7 +1798,7 @@ static int devlink_dpipe_tables_fill(struct genl_info *info,
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
- goto err_skb_send_alloc;
+ return err;
goto send_done;
}
@@ -1807,7 +1807,6 @@ static int devlink_dpipe_tables_fill(struct genl_info *info,
nla_put_failure:
err = -EMSGSIZE;
err_table_put:
-err_skb_send_alloc:
genlmsg_cancel(skb, hdr);
nlmsg_free(skb);
return err;
@@ -2073,7 +2072,7 @@ static int devlink_dpipe_entries_fill(struct genl_info *info,
table->counters_enabled,
&dump_ctx);
if (err)
- goto err_entries_dump;
+ return err;
send_done:
nlh = nlmsg_put(dump_ctx.skb, info->snd_portid, info->snd_seq,
@@ -2081,16 +2080,10 @@ static int devlink_dpipe_entries_fill(struct genl_info *info,
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&dump_ctx.skb, info);
if (err)
- goto err_skb_send_alloc;
+ return err;
goto send_done;
}
return genlmsg_reply(dump_ctx.skb, info);
-
-err_entries_dump:
-err_skb_send_alloc:
- genlmsg_cancel(dump_ctx.skb, dump_ctx.hdr);
- nlmsg_free(dump_ctx.skb);
- return err;
}
static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb,
@@ -2229,7 +2222,7 @@ static int devlink_dpipe_headers_fill(struct genl_info *info,
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
- goto err_skb_send_alloc;
+ return err;
goto send_done;
}
return genlmsg_reply(skb, info);
@@ -2237,7 +2230,6 @@ static int devlink_dpipe_headers_fill(struct genl_info *info,
nla_put_failure:
err = -EMSGSIZE;
err_table_put:
-err_skb_send_alloc:
genlmsg_cancel(skb, hdr);
nlmsg_free(skb);
return err;
--
2.4.11
^ permalink raw reply related
* Re: HW question: i210 vs. BCM5461S over SGMII: no response from PHY to MDIO requests?
From: Andrew Lunn @ 2018-03-18 14:40 UTC (permalink / raw)
To: Frantisek Rysanek; +Cc: netdev
In-Reply-To: <5AAD92D1.16223.367B9940@Frantisek.Rysanek.post.cz>
> I'm not getting an ACK from the SFP, probably because I've got the
> address and offset wrong and because I'd better use indirect access.
> There's some more work awaiting me...
Try address 0x50.
i2detect will probe all addresses for you, if you have a standard
Linux i2c bus.
Andrew
^ permalink raw reply
* Re: [PATCH v11 crypto 00/12] Chelsio Inline TLS
From: David Miller @ 2018-03-18 14:36 UTC (permalink / raw)
To: atul.gupta
Cc: davejwatson, herbert, sd, sbrivio, linux-crypto, netdev, ganeshgr
In-Reply-To: <CY4PR1201MB0230F43EF6E3AD3DA253FE0B97D50@CY4PR1201MB0230.namprd12.prod.outlook.com>
From: Atul Gupta <atul.gupta@chelsio.com>
Date: Sun, 18 Mar 2018 14:30:30 +0000
> Hi Dave/Herbert,
>
> This series is against crypto tree, should I submit two patch series:
> 1. netdev specific changes against net-next tree?
> 2. crypto changes against crypto tree?
Herbert, is it OK for this entire series to go via net-next?
Thanks!
^ permalink raw reply
* RE: [PATCH v11 crypto 00/12] Chelsio Inline TLS
From: Atul Gupta @ 2018-03-18 14:30 UTC (permalink / raw)
To: David Miller
Cc: davejwatson@fb.com, herbert@gondor.apana.org.au,
sd@queasysnail.net, sbrivio@redhat.com,
linux-crypto@vger.kernel.org, netdev@vger.kernel.org, Ganesh GR
In-Reply-To: <20180317.200250.1382959658122666064.davem@davemloft.net>
Hi Dave/Herbert,
This series is against crypto tree, should I submit two patch series:
1. netdev specific changes against net-next tree?
2. crypto changes against crypto tree?
Regards
Atul
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Sunday, March 18, 2018 5:33 AM
To: Atul Gupta <atul.gupta@chelsio.com>
Cc: davejwatson@fb.com; herbert@gondor.apana.org.au; sd@queasysnail.net; sbrivio@redhat.com; linux-crypto@vger.kernel.org; netdev@vger.kernel.org; Ganesh GR <ganeshgr@chelsio.com>
Subject: Re: [PATCH v11 crypto 00/12] Chelsio Inline TLS
From: Atul Gupta <atul.gupta@chelsio.com>
Date: Fri, 16 Mar 2018 21:06:22 +0530
> Series for Chelsio Inline TLS driver (chtls)
This series doesn't even come close to applying to the net-next tree, please respin.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: dsa: drop some VLAs in switch.c
From: Salvatore Mesoraca @ 2018-03-18 14:08 UTC (permalink / raw)
To: David Laight
Cc: Vivien Didelot, linux-kernel@vger.kernel.org, Kernel Hardening,
netdev@vger.kernel.org, David S. Miller, Andrew Lunn,
Florian Fainelli, Kees Cook
In-Reply-To: <CAJHCu1LB1wPErEB0xNd_9=_8dmOn=VFXmzu=-U5Eg3zTeT4b6g@mail.gmail.com>
2018-03-14 13:48 GMT+01:00 Salvatore Mesoraca <s.mesoraca16@gmail.com>:
> 2018-03-14 12:24 GMT+01:00 David Laight <David.Laight@aculab.com>:
>> Isn't using DECLARE_BITMAP() completely OTT when the maximum size is less
>> than the number of bits in a word?
>
> It allocates ceiling(size/8) "unsigned long"s, so yes.
Actually I meant ceiling(size/8/sizeof(unsigned long))
I'm sorry for the typo.
Salvatore
^ permalink raw reply
* Re: [PATCH v2 1/2] sysfs: symlink: export sysfs_create_link_nowarn()
From: Greg Kroah-Hartman @ 2018-03-18 13:13 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Andrew Lunn, Florian Fainelli,
Sekhar Nori, linux-kernel, linux-omap
In-Reply-To: <20180316220835.30006-2-grygorii.strashko@ti.com>
On Fri, Mar 16, 2018 at 05:08:34PM -0500, Grygorii Strashko wrote:
> The sysfs_create_link_nowarn() is going to be used in phylib framework in
> subsequent patch which can be built as module. Hence, export
> sysfs_create_link_nowarn() to avoid build errors.
>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Fixes: a3995460491d ("net: phy: Relax error checking on sysfs_create_link()")
This specific patch doesn't fix anything, it just _allows_ it to be
fixed in the second patch :)
Anyway, just a nit...
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [PATCH] vmxnet3: fix LRO feature check
From: kbuild test robot @ 2018-03-18 11:23 UTC (permalink / raw)
To: Igor Pylypiv
Cc: kbuild-all, Shrikrishna Khare, VMware, Inc., netdev, ipylypiv
In-Reply-To: <20180317075852.11785-1-ipylypiv@silver-peak.com>
[-- Attachment #1: Type: text/plain, Size: 12414 bytes --]
Hi Igor,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Igor-Pylypiv/vmxnet3-fix-LRO-feature-check/20180318-140725
config: x86_64-randconfig-s3-03181820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/net//vmxnet3/vmxnet3_drv.c:27:
drivers/net//vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_rq_rx_complete':
drivers/net//vmxnet3/vmxnet3_drv.c:1474:8: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' [-Wparentheses]
!adapter->netdev->features & NETIF_F_LRO) {
^~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/net//vmxnet3/vmxnet3_drv.c:1473:4: note: in expansion of macro 'if'
if (!rcd->tcp ||
^~
drivers/net//vmxnet3/vmxnet3_drv.c:1474:8: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' [-Wparentheses]
!adapter->netdev->features & NETIF_F_LRO) {
^~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/net//vmxnet3/vmxnet3_drv.c:1473:4: note: in expansion of macro 'if'
if (!rcd->tcp ||
^~
drivers/net//vmxnet3/vmxnet3_drv.c:1474:8: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' [-Wparentheses]
!adapter->netdev->features & NETIF_F_LRO) {
^~~~~~~~~~~~~~~~~~~
include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> drivers/net//vmxnet3/vmxnet3_drv.c:1473:4: note: in expansion of macro 'if'
if (!rcd->tcp ||
^~
vim +/if +1473 drivers/net//vmxnet3/vmxnet3_drv.c
1255
1256 static int
1257 vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
1258 struct vmxnet3_adapter *adapter, int quota)
1259 {
1260 static const u32 rxprod_reg[2] = {
1261 VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2
1262 };
1263 u32 num_pkts = 0;
1264 bool skip_page_frags = false;
1265 struct Vmxnet3_RxCompDesc *rcd;
1266 struct vmxnet3_rx_ctx *ctx = &rq->rx_ctx;
1267 u16 segCnt = 0, mss = 0;
1268 #ifdef __BIG_ENDIAN_BITFIELD
1269 struct Vmxnet3_RxDesc rxCmdDesc;
1270 struct Vmxnet3_RxCompDesc rxComp;
1271 #endif
1272 vmxnet3_getRxComp(rcd, &rq->comp_ring.base[rq->comp_ring.next2proc].rcd,
1273 &rxComp);
1274 while (rcd->gen == rq->comp_ring.gen) {
1275 struct vmxnet3_rx_buf_info *rbi;
1276 struct sk_buff *skb, *new_skb = NULL;
1277 struct page *new_page = NULL;
1278 dma_addr_t new_dma_addr;
1279 int num_to_alloc;
1280 struct Vmxnet3_RxDesc *rxd;
1281 u32 idx, ring_idx;
1282 struct vmxnet3_cmd_ring *ring = NULL;
1283 if (num_pkts >= quota) {
1284 /* we may stop even before we see the EOP desc of
1285 * the current pkt
1286 */
1287 break;
1288 }
1289 BUG_ON(rcd->rqID != rq->qid && rcd->rqID != rq->qid2 &&
1290 rcd->rqID != rq->dataRingQid);
1291 idx = rcd->rxdIdx;
1292 ring_idx = VMXNET3_GET_RING_IDX(adapter, rcd->rqID);
1293 ring = rq->rx_ring + ring_idx;
1294 vmxnet3_getRxDesc(rxd, &rq->rx_ring[ring_idx].base[idx].rxd,
1295 &rxCmdDesc);
1296 rbi = rq->buf_info[ring_idx] + idx;
1297
1298 BUG_ON(rxd->addr != rbi->dma_addr ||
1299 rxd->len != rbi->len);
1300
1301 if (unlikely(rcd->eop && rcd->err)) {
1302 vmxnet3_rx_error(rq, rcd, ctx, adapter);
1303 goto rcd_done;
1304 }
1305
1306 if (rcd->sop) { /* first buf of the pkt */
1307 bool rxDataRingUsed;
1308 u16 len;
1309
1310 BUG_ON(rxd->btype != VMXNET3_RXD_BTYPE_HEAD ||
1311 (rcd->rqID != rq->qid &&
1312 rcd->rqID != rq->dataRingQid));
1313
1314 BUG_ON(rbi->buf_type != VMXNET3_RX_BUF_SKB);
1315 BUG_ON(ctx->skb != NULL || rbi->skb == NULL);
1316
1317 if (unlikely(rcd->len == 0)) {
1318 /* Pretend the rx buffer is skipped. */
1319 BUG_ON(!(rcd->sop && rcd->eop));
1320 netdev_dbg(adapter->netdev,
1321 "rxRing[%u][%u] 0 length\n",
1322 ring_idx, idx);
1323 goto rcd_done;
1324 }
1325
1326 skip_page_frags = false;
1327 ctx->skb = rbi->skb;
1328
1329 rxDataRingUsed =
1330 VMXNET3_RX_DATA_RING(adapter, rcd->rqID);
1331 len = rxDataRingUsed ? rcd->len : rbi->len;
1332 new_skb = netdev_alloc_skb_ip_align(adapter->netdev,
1333 len);
1334 if (new_skb == NULL) {
1335 /* Skb allocation failed, do not handover this
1336 * skb to stack. Reuse it. Drop the existing pkt
1337 */
1338 rq->stats.rx_buf_alloc_failure++;
1339 ctx->skb = NULL;
1340 rq->stats.drop_total++;
1341 skip_page_frags = true;
1342 goto rcd_done;
1343 }
1344
1345 if (rxDataRingUsed) {
1346 size_t sz;
1347
1348 BUG_ON(rcd->len > rq->data_ring.desc_size);
1349
1350 ctx->skb = new_skb;
1351 sz = rcd->rxdIdx * rq->data_ring.desc_size;
1352 memcpy(new_skb->data,
1353 &rq->data_ring.base[sz], rcd->len);
1354 } else {
1355 ctx->skb = rbi->skb;
1356
1357 new_dma_addr =
1358 dma_map_single(&adapter->pdev->dev,
1359 new_skb->data, rbi->len,
1360 PCI_DMA_FROMDEVICE);
1361 if (dma_mapping_error(&adapter->pdev->dev,
1362 new_dma_addr)) {
1363 dev_kfree_skb(new_skb);
1364 /* Skb allocation failed, do not
1365 * handover this skb to stack. Reuse
1366 * it. Drop the existing pkt.
1367 */
1368 rq->stats.rx_buf_alloc_failure++;
1369 ctx->skb = NULL;
1370 rq->stats.drop_total++;
1371 skip_page_frags = true;
1372 goto rcd_done;
1373 }
1374
1375 dma_unmap_single(&adapter->pdev->dev,
1376 rbi->dma_addr,
1377 rbi->len,
1378 PCI_DMA_FROMDEVICE);
1379
1380 /* Immediate refill */
1381 rbi->skb = new_skb;
1382 rbi->dma_addr = new_dma_addr;
1383 rxd->addr = cpu_to_le64(rbi->dma_addr);
1384 rxd->len = rbi->len;
1385 }
1386
1387 #ifdef VMXNET3_RSS
1388 if (rcd->rssType != VMXNET3_RCD_RSS_TYPE_NONE &&
1389 (adapter->netdev->features & NETIF_F_RXHASH))
1390 skb_set_hash(ctx->skb,
1391 le32_to_cpu(rcd->rssHash),
1392 PKT_HASH_TYPE_L3);
1393 #endif
1394 skb_put(ctx->skb, rcd->len);
1395
1396 if (VMXNET3_VERSION_GE_2(adapter) &&
1397 rcd->type == VMXNET3_CDTYPE_RXCOMP_LRO) {
1398 struct Vmxnet3_RxCompDescExt *rcdlro;
1399 rcdlro = (struct Vmxnet3_RxCompDescExt *)rcd;
1400
1401 segCnt = rcdlro->segCnt;
1402 WARN_ON_ONCE(segCnt == 0);
1403 mss = rcdlro->mss;
1404 if (unlikely(segCnt <= 1))
1405 segCnt = 0;
1406 } else {
1407 segCnt = 0;
1408 }
1409 } else {
1410 BUG_ON(ctx->skb == NULL && !skip_page_frags);
1411
1412 /* non SOP buffer must be type 1 in most cases */
1413 BUG_ON(rbi->buf_type != VMXNET3_RX_BUF_PAGE);
1414 BUG_ON(rxd->btype != VMXNET3_RXD_BTYPE_BODY);
1415
1416 /* If an sop buffer was dropped, skip all
1417 * following non-sop fragments. They will be reused.
1418 */
1419 if (skip_page_frags)
1420 goto rcd_done;
1421
1422 if (rcd->len) {
1423 new_page = alloc_page(GFP_ATOMIC);
1424 /* Replacement page frag could not be allocated.
1425 * Reuse this page. Drop the pkt and free the
1426 * skb which contained this page as a frag. Skip
1427 * processing all the following non-sop frags.
1428 */
1429 if (unlikely(!new_page)) {
1430 rq->stats.rx_buf_alloc_failure++;
1431 dev_kfree_skb(ctx->skb);
1432 ctx->skb = NULL;
1433 skip_page_frags = true;
1434 goto rcd_done;
1435 }
1436 new_dma_addr = dma_map_page(&adapter->pdev->dev,
1437 new_page,
1438 0, PAGE_SIZE,
1439 PCI_DMA_FROMDEVICE);
1440 if (dma_mapping_error(&adapter->pdev->dev,
1441 new_dma_addr)) {
1442 put_page(new_page);
1443 rq->stats.rx_buf_alloc_failure++;
1444 dev_kfree_skb(ctx->skb);
1445 ctx->skb = NULL;
1446 skip_page_frags = true;
1447 goto rcd_done;
1448 }
1449
1450 dma_unmap_page(&adapter->pdev->dev,
1451 rbi->dma_addr, rbi->len,
1452 PCI_DMA_FROMDEVICE);
1453
1454 vmxnet3_append_frag(ctx->skb, rcd, rbi);
1455
1456 /* Immediate refill */
1457 rbi->page = new_page;
1458 rbi->dma_addr = new_dma_addr;
1459 rxd->addr = cpu_to_le64(rbi->dma_addr);
1460 rxd->len = rbi->len;
1461 }
1462 }
1463
1464
1465 skb = ctx->skb;
1466 if (rcd->eop) {
1467 u32 mtu = adapter->netdev->mtu;
1468 skb->len += skb->data_len;
1469
1470 vmxnet3_rx_csum(adapter, skb,
1471 (union Vmxnet3_GenericDesc *)rcd);
1472 skb->protocol = eth_type_trans(skb, adapter->netdev);
> 1473 if (!rcd->tcp ||
1474 !adapter->netdev->features & NETIF_F_LRO) {
1475 goto not_lro;
1476 }
1477
1478 if (segCnt != 0 && mss != 0) {
1479 skb_shinfo(skb)->gso_type = rcd->v4 ?
1480 SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
1481 skb_shinfo(skb)->gso_size = mss;
1482 skb_shinfo(skb)->gso_segs = segCnt;
1483 } else if (segCnt != 0 || skb->len > mtu) {
1484 u32 hlen;
1485
1486 hlen = vmxnet3_get_hdr_len(adapter, skb,
1487 (union Vmxnet3_GenericDesc *)rcd);
1488 if (hlen == 0)
1489 goto not_lro;
1490
1491 skb_shinfo(skb)->gso_type =
1492 rcd->v4 ? SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
1493 if (segCnt != 0) {
1494 skb_shinfo(skb)->gso_segs = segCnt;
1495 skb_shinfo(skb)->gso_size =
1496 DIV_ROUND_UP(skb->len -
1497 hlen, segCnt);
1498 } else {
1499 skb_shinfo(skb)->gso_size = mtu - hlen;
1500 }
1501 }
1502 not_lro:
1503 if (unlikely(rcd->ts))
1504 __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), rcd->tci);
1505
1506 if (adapter->netdev->features & NETIF_F_LRO)
1507 netif_receive_skb(skb);
1508 else
1509 napi_gro_receive(&rq->napi, skb);
1510
1511 ctx->skb = NULL;
1512 num_pkts++;
1513 }
1514
1515 rcd_done:
1516 /* device may have skipped some rx descs */
1517 ring->next2comp = idx;
1518 num_to_alloc = vmxnet3_cmd_ring_desc_avail(ring);
1519 ring = rq->rx_ring + ring_idx;
1520 while (num_to_alloc) {
1521 vmxnet3_getRxDesc(rxd, &ring->base[ring->next2fill].rxd,
1522 &rxCmdDesc);
1523 BUG_ON(!rxd->addr);
1524
1525 /* Recv desc is ready to be used by the device */
1526 rxd->gen = ring->gen;
1527 vmxnet3_cmd_ring_adv_next2fill(ring);
1528 num_to_alloc--;
1529 }
1530
1531 /* if needed, update the register */
1532 if (unlikely(rq->shared->updateRxProd)) {
1533 VMXNET3_WRITE_BAR0_REG(adapter,
1534 rxprod_reg[ring_idx] + rq->qid * 8,
1535 ring->next2fill);
1536 }
1537
1538 vmxnet3_comp_ring_adv_next2proc(&rq->comp_ring);
1539 vmxnet3_getRxComp(rcd,
1540 &rq->comp_ring.base[rq->comp_ring.next2proc].rcd, &rxComp);
1541 }
1542
1543 return num_pkts;
1544 }
1545
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27875 bytes --]
^ permalink raw reply
* linux-next on x60: network manager often complains "network is disabled" after resume
From: Pavel Machek @ 2018-03-18 10:40 UTC (permalink / raw)
To: Rafael J. Wysocki, kernel list, Linux-pm mailing list; +Cc: Netdev list
[-- Attachment #1: Type: text/plain, Size: 331 bytes --]
Hi!
With recent linux-next, after resume networkmanager often claims that
"network is disabled". Sometimes suspend/resume clears that.
Any ideas? Does it work for you?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 03/12] dt-bindings: net: dwmac-sun8i: Clean up clock delay chain descriptions
From: Sergei Shtylyov @ 2018-03-18 9:21 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard, Michael Turquette, Stephen Boyd,
Giuseppe Cavallaro, Rob Herring, Mark Rutland, Mark Brown
Cc: devicetree, netdev, Corentin Labbe, linux-clk, linux-arm-kernel,
Icenowy Zheng
In-Reply-To: <20180317092857.4396-4-wens@csie.org>
Hello!
On 3/17/2018 12:28 PM, Chen-Yu Tsai wrote:
> The clock delay chains found in the glue layer for dwmac-sun8i are only
> used with RGMII PHYs. They are not intended for non-RGMII PHYs, such as
> MII external PHYs or the internal PHY. Also, a recent SoC has a smaller
> range of possible values for the delay chain.
>
> This patch reformats the delay chain section of the device tree binding
> to make it clear that the delay chains only apply to RGMII PHYs, and
> make it easier to add the R40-specific bits later.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 3d6d5fa0c4d5..b8a3028d6c30 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -28,10 +28,13 @@ Required properties:
> - allwinner,sun8i-a83t-system-controller
>
> Optional properties:
> -- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 0-700. Default is 0)
> -- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 0-3100. Default is 0)
> -Both delay properties need to be a multiple of 100. They control the delay for
> -external PHY.
> +- allwinner,tx-delay-ps: TX clock delay chain value in ps.
> + Range is 0-700. Default is 0.
> +- allwinner,rx-delay-ps: RX clock delay chain value in ps.
> + Range is 0-3100. Default is 0.
> +Both delay properties need to be a multiple of 100. They control the
> +clock delay for external RGMII PHY. They are do apply to the internal
s/are do/do not/?
> +PHY or external non-RGMII PHYs.
>
> Optional properties for the following compatibles:
> - "allwinner,sun8i-h3-emac",
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] json_print: fix print_uint with helper type extensions
From: Kevin Darbyshire-Bryant @ 2018-03-18 8:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180316133437.5f535a0e@xeon-e3>
[-- Attachment #1: Type: text/plain, Size: 853 bytes --]
> On 16 Mar 2018, at 20:34, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>
>> print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)
>>
>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
>
> I am fine with this. But since there is no code using it yet, it should
> go net-next branch.
>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
Existing code is tripping up over the hidden uint - > uint64_t promotion in print_uint in iproute2 v4.15, that’s how I fell over the issue. Should I split the patch? One fixing the uint->uint64_t and the other offering the explicit type length options.
Obviously I now realise that the email header should have iproute2 in it. Learning, slowly :-)
Cheers,
Kevin D-B
012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] vmxnet3: fix LRO feature check
From: kbuild test robot @ 2018-03-18 6:37 UTC (permalink / raw)
To: Igor Pylypiv
Cc: kbuild-all, Shrikrishna Khare, VMware, Inc., netdev, ipylypiv
In-Reply-To: <20180317075852.11785-1-ipylypiv@silver-peak.com>
[-- Attachment #1: Type: text/plain, Size: 10893 bytes --]
Hi Igor,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Igor-Pylypiv/vmxnet3-fix-LRO-feature-check/20180318-140725
config: i386-randconfig-x003-201811 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/net/vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_rq_rx_complete':
>> drivers/net/vmxnet3/vmxnet3_drv.c:1474:8: warning: suggest parentheses around operand of '!' or change '&' to '&&' or '!' to '~' [-Wparentheses]
!adapter->netdev->features & NETIF_F_LRO) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +1474 drivers/net/vmxnet3/vmxnet3_drv.c
1255
1256 static int
1257 vmxnet3_rq_rx_complete(struct vmxnet3_rx_queue *rq,
1258 struct vmxnet3_adapter *adapter, int quota)
1259 {
1260 static const u32 rxprod_reg[2] = {
1261 VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2
1262 };
1263 u32 num_pkts = 0;
1264 bool skip_page_frags = false;
1265 struct Vmxnet3_RxCompDesc *rcd;
1266 struct vmxnet3_rx_ctx *ctx = &rq->rx_ctx;
1267 u16 segCnt = 0, mss = 0;
1268 #ifdef __BIG_ENDIAN_BITFIELD
1269 struct Vmxnet3_RxDesc rxCmdDesc;
1270 struct Vmxnet3_RxCompDesc rxComp;
1271 #endif
1272 vmxnet3_getRxComp(rcd, &rq->comp_ring.base[rq->comp_ring.next2proc].rcd,
1273 &rxComp);
1274 while (rcd->gen == rq->comp_ring.gen) {
1275 struct vmxnet3_rx_buf_info *rbi;
1276 struct sk_buff *skb, *new_skb = NULL;
1277 struct page *new_page = NULL;
1278 dma_addr_t new_dma_addr;
1279 int num_to_alloc;
1280 struct Vmxnet3_RxDesc *rxd;
1281 u32 idx, ring_idx;
1282 struct vmxnet3_cmd_ring *ring = NULL;
1283 if (num_pkts >= quota) {
1284 /* we may stop even before we see the EOP desc of
1285 * the current pkt
1286 */
1287 break;
1288 }
1289 BUG_ON(rcd->rqID != rq->qid && rcd->rqID != rq->qid2 &&
1290 rcd->rqID != rq->dataRingQid);
1291 idx = rcd->rxdIdx;
1292 ring_idx = VMXNET3_GET_RING_IDX(adapter, rcd->rqID);
1293 ring = rq->rx_ring + ring_idx;
1294 vmxnet3_getRxDesc(rxd, &rq->rx_ring[ring_idx].base[idx].rxd,
1295 &rxCmdDesc);
1296 rbi = rq->buf_info[ring_idx] + idx;
1297
1298 BUG_ON(rxd->addr != rbi->dma_addr ||
1299 rxd->len != rbi->len);
1300
1301 if (unlikely(rcd->eop && rcd->err)) {
1302 vmxnet3_rx_error(rq, rcd, ctx, adapter);
1303 goto rcd_done;
1304 }
1305
1306 if (rcd->sop) { /* first buf of the pkt */
1307 bool rxDataRingUsed;
1308 u16 len;
1309
1310 BUG_ON(rxd->btype != VMXNET3_RXD_BTYPE_HEAD ||
1311 (rcd->rqID != rq->qid &&
1312 rcd->rqID != rq->dataRingQid));
1313
1314 BUG_ON(rbi->buf_type != VMXNET3_RX_BUF_SKB);
1315 BUG_ON(ctx->skb != NULL || rbi->skb == NULL);
1316
1317 if (unlikely(rcd->len == 0)) {
1318 /* Pretend the rx buffer is skipped. */
1319 BUG_ON(!(rcd->sop && rcd->eop));
1320 netdev_dbg(adapter->netdev,
1321 "rxRing[%u][%u] 0 length\n",
1322 ring_idx, idx);
1323 goto rcd_done;
1324 }
1325
1326 skip_page_frags = false;
1327 ctx->skb = rbi->skb;
1328
1329 rxDataRingUsed =
1330 VMXNET3_RX_DATA_RING(adapter, rcd->rqID);
1331 len = rxDataRingUsed ? rcd->len : rbi->len;
1332 new_skb = netdev_alloc_skb_ip_align(adapter->netdev,
1333 len);
1334 if (new_skb == NULL) {
1335 /* Skb allocation failed, do not handover this
1336 * skb to stack. Reuse it. Drop the existing pkt
1337 */
1338 rq->stats.rx_buf_alloc_failure++;
1339 ctx->skb = NULL;
1340 rq->stats.drop_total++;
1341 skip_page_frags = true;
1342 goto rcd_done;
1343 }
1344
1345 if (rxDataRingUsed) {
1346 size_t sz;
1347
1348 BUG_ON(rcd->len > rq->data_ring.desc_size);
1349
1350 ctx->skb = new_skb;
1351 sz = rcd->rxdIdx * rq->data_ring.desc_size;
1352 memcpy(new_skb->data,
1353 &rq->data_ring.base[sz], rcd->len);
1354 } else {
1355 ctx->skb = rbi->skb;
1356
1357 new_dma_addr =
1358 dma_map_single(&adapter->pdev->dev,
1359 new_skb->data, rbi->len,
1360 PCI_DMA_FROMDEVICE);
1361 if (dma_mapping_error(&adapter->pdev->dev,
1362 new_dma_addr)) {
1363 dev_kfree_skb(new_skb);
1364 /* Skb allocation failed, do not
1365 * handover this skb to stack. Reuse
1366 * it. Drop the existing pkt.
1367 */
1368 rq->stats.rx_buf_alloc_failure++;
1369 ctx->skb = NULL;
1370 rq->stats.drop_total++;
1371 skip_page_frags = true;
1372 goto rcd_done;
1373 }
1374
1375 dma_unmap_single(&adapter->pdev->dev,
1376 rbi->dma_addr,
1377 rbi->len,
1378 PCI_DMA_FROMDEVICE);
1379
1380 /* Immediate refill */
1381 rbi->skb = new_skb;
1382 rbi->dma_addr = new_dma_addr;
1383 rxd->addr = cpu_to_le64(rbi->dma_addr);
1384 rxd->len = rbi->len;
1385 }
1386
1387 #ifdef VMXNET3_RSS
1388 if (rcd->rssType != VMXNET3_RCD_RSS_TYPE_NONE &&
1389 (adapter->netdev->features & NETIF_F_RXHASH))
1390 skb_set_hash(ctx->skb,
1391 le32_to_cpu(rcd->rssHash),
1392 PKT_HASH_TYPE_L3);
1393 #endif
1394 skb_put(ctx->skb, rcd->len);
1395
1396 if (VMXNET3_VERSION_GE_2(adapter) &&
1397 rcd->type == VMXNET3_CDTYPE_RXCOMP_LRO) {
1398 struct Vmxnet3_RxCompDescExt *rcdlro;
1399 rcdlro = (struct Vmxnet3_RxCompDescExt *)rcd;
1400
1401 segCnt = rcdlro->segCnt;
1402 WARN_ON_ONCE(segCnt == 0);
1403 mss = rcdlro->mss;
1404 if (unlikely(segCnt <= 1))
1405 segCnt = 0;
1406 } else {
1407 segCnt = 0;
1408 }
1409 } else {
1410 BUG_ON(ctx->skb == NULL && !skip_page_frags);
1411
1412 /* non SOP buffer must be type 1 in most cases */
1413 BUG_ON(rbi->buf_type != VMXNET3_RX_BUF_PAGE);
1414 BUG_ON(rxd->btype != VMXNET3_RXD_BTYPE_BODY);
1415
1416 /* If an sop buffer was dropped, skip all
1417 * following non-sop fragments. They will be reused.
1418 */
1419 if (skip_page_frags)
1420 goto rcd_done;
1421
1422 if (rcd->len) {
1423 new_page = alloc_page(GFP_ATOMIC);
1424 /* Replacement page frag could not be allocated.
1425 * Reuse this page. Drop the pkt and free the
1426 * skb which contained this page as a frag. Skip
1427 * processing all the following non-sop frags.
1428 */
1429 if (unlikely(!new_page)) {
1430 rq->stats.rx_buf_alloc_failure++;
1431 dev_kfree_skb(ctx->skb);
1432 ctx->skb = NULL;
1433 skip_page_frags = true;
1434 goto rcd_done;
1435 }
1436 new_dma_addr = dma_map_page(&adapter->pdev->dev,
1437 new_page,
1438 0, PAGE_SIZE,
1439 PCI_DMA_FROMDEVICE);
1440 if (dma_mapping_error(&adapter->pdev->dev,
1441 new_dma_addr)) {
1442 put_page(new_page);
1443 rq->stats.rx_buf_alloc_failure++;
1444 dev_kfree_skb(ctx->skb);
1445 ctx->skb = NULL;
1446 skip_page_frags = true;
1447 goto rcd_done;
1448 }
1449
1450 dma_unmap_page(&adapter->pdev->dev,
1451 rbi->dma_addr, rbi->len,
1452 PCI_DMA_FROMDEVICE);
1453
1454 vmxnet3_append_frag(ctx->skb, rcd, rbi);
1455
1456 /* Immediate refill */
1457 rbi->page = new_page;
1458 rbi->dma_addr = new_dma_addr;
1459 rxd->addr = cpu_to_le64(rbi->dma_addr);
1460 rxd->len = rbi->len;
1461 }
1462 }
1463
1464
1465 skb = ctx->skb;
1466 if (rcd->eop) {
1467 u32 mtu = adapter->netdev->mtu;
1468 skb->len += skb->data_len;
1469
1470 vmxnet3_rx_csum(adapter, skb,
1471 (union Vmxnet3_GenericDesc *)rcd);
1472 skb->protocol = eth_type_trans(skb, adapter->netdev);
1473 if (!rcd->tcp ||
> 1474 !adapter->netdev->features & NETIF_F_LRO) {
1475 goto not_lro;
1476 }
1477
1478 if (segCnt != 0 && mss != 0) {
1479 skb_shinfo(skb)->gso_type = rcd->v4 ?
1480 SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
1481 skb_shinfo(skb)->gso_size = mss;
1482 skb_shinfo(skb)->gso_segs = segCnt;
1483 } else if (segCnt != 0 || skb->len > mtu) {
1484 u32 hlen;
1485
1486 hlen = vmxnet3_get_hdr_len(adapter, skb,
1487 (union Vmxnet3_GenericDesc *)rcd);
1488 if (hlen == 0)
1489 goto not_lro;
1490
1491 skb_shinfo(skb)->gso_type =
1492 rcd->v4 ? SKB_GSO_TCPV4 : SKB_GSO_TCPV6;
1493 if (segCnt != 0) {
1494 skb_shinfo(skb)->gso_segs = segCnt;
1495 skb_shinfo(skb)->gso_size =
1496 DIV_ROUND_UP(skb->len -
1497 hlen, segCnt);
1498 } else {
1499 skb_shinfo(skb)->gso_size = mtu - hlen;
1500 }
1501 }
1502 not_lro:
1503 if (unlikely(rcd->ts))
1504 __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), rcd->tci);
1505
1506 if (adapter->netdev->features & NETIF_F_LRO)
1507 netif_receive_skb(skb);
1508 else
1509 napi_gro_receive(&rq->napi, skb);
1510
1511 ctx->skb = NULL;
1512 num_pkts++;
1513 }
1514
1515 rcd_done:
1516 /* device may have skipped some rx descs */
1517 ring->next2comp = idx;
1518 num_to_alloc = vmxnet3_cmd_ring_desc_avail(ring);
1519 ring = rq->rx_ring + ring_idx;
1520 while (num_to_alloc) {
1521 vmxnet3_getRxDesc(rxd, &ring->base[ring->next2fill].rxd,
1522 &rxCmdDesc);
1523 BUG_ON(!rxd->addr);
1524
1525 /* Recv desc is ready to be used by the device */
1526 rxd->gen = ring->gen;
1527 vmxnet3_cmd_ring_adv_next2fill(ring);
1528 num_to_alloc--;
1529 }
1530
1531 /* if needed, update the register */
1532 if (unlikely(rq->shared->updateRxProd)) {
1533 VMXNET3_WRITE_BAR0_REG(adapter,
1534 rxprod_reg[ring_idx] + rq->qid * 8,
1535 ring->next2fill);
1536 }
1537
1538 vmxnet3_comp_ring_adv_next2proc(&rq->comp_ring);
1539 vmxnet3_getRxComp(rcd,
1540 &rq->comp_ring.base[rq->comp_ring.next2proc].rcd, &rxComp);
1541 }
1542
1543 return num_pkts;
1544 }
1545
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34241 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 05/10] phy: cp110-comphy: 2.5G SGMII mode
From: Baruch Siach @ 2018-03-18 4:42 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, kishon, linux, gregory.clement, andrew, jason,
sebastian.hesselbarth, ymarkman, netdev, linux-kernel,
maxime.chevallier, nadavh, thomas.petazzoni, miquel.raynal,
stefanc, mw, linux-arm-kernel
In-Reply-To: <20180316103351.16616-6-antoine.tenart@bootlin.com>
Hi Antoine,
On Fri, Mar 16, 2018 at 11:33:46AM +0100, Antoine Tenart wrote:
> This patch allow the CP100 comphy to configure some lanes in the
Should be 'CP110'.
> 2.5G SGMII mode. This mode is quite close to SGMII and uses nearly the
> same code path.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* [PATCH] vmxnet3: remove unused flag "rxcsum" from struct vmxnet3_adapter
From: Igor Pylypiv @ 2018-03-18 1:17 UTC (permalink / raw)
To: David Miller; +Cc: Shrikrishna Khare, VMware, Inc., netdev, ipylypiv
Signed-off-by: Igor Pylypiv <ipylypiv@silver-peak.com>
---
drivers/net/vmxnet3/vmxnet3_int.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 018375f5d108..3de4cecda35a 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -342,8 +342,6 @@ struct vmxnet3_adapter {
u8 __iomem *hw_addr1; /* for BAR 1 */
u8 version;
- bool rxcsum;
-
#ifdef VMXNET3_RSS
struct UPT1_RSSConf *rss_conf;
bool rss;
--
2.16.2
^ permalink raw reply related
* Re: [PATCH] vmxnet3: fix LRO feature check
From: Igor Pylypiv @ 2018-03-18 1:08 UTC (permalink / raw)
To: David Miller; +Cc: skhare, pv-drivers, netdev
In-Reply-To: <20180317.202032.2223279074951132662.davem@davemloft.net>
The 03/17/2018 20:20, David Miller wrote:
> From: Igor Pylypiv <ipylypiv@silver-peak.com>
> Date: Sat, 17 Mar 2018 00:58:52 -0700
>
> > rxcsum and lro fields were deleted in commit a0d2730c9571 ("net: vmxnet3:
> > convert to hw_features"). With upgrading to newer version those fields were
> > resurrected and new code started using uninitialized lro field.
> > Removing rxcsum and lro fields.
> >
> > Fixes: 45dac1d6ea04 ("vmxnet3: Changes for vmxnet3 adapter version 2 (fwd)")
> > Signed-off-by: Igor Pylypiv <ipylypiv@silver-peak.com>
>
> Why are you posting this again?
>
> I applied the copy of this patch which was part of a two part series
> posted earlier.
No way!!!
I know it is hard to believe but I found this bug myself.
That's really odd. I took linux-next-20180316 and patch wasn't there.
Now I see the patch in your tree.
Anyway, let me send another one to delete rxcsum at least...
^ 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