* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Jamal Hadi Salim @ 2018-09-18 12:55 UTC (permalink / raw)
To: Johannes Berg, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <1537274378.2957.23.camel@sipsolutions.net>
On 2018-09-18 8:39 a.m., Johannes Berg wrote:
> On Tue, 2018-09-18 at 08:34 -0400, Jamal Hadi Salim wrote:
>
>> Maybe time to introduce kernel side access-control flags?
>> Read/Write permissions for example. Attrs marked as read only
>> (in the kernel) cannot be written to.
>
> I dunno, that might work for ethtool, but I want to use it for something
> that's not even an attribute you could think about writing to, but the
> result of some operation you started.
>
Execute permission kind of thing? i.e if i understood you correctly
if acl is "rwx" then attribute can only be written to (or read from) if
the "thing executing" is complete
> What would the practical difference be though? Hopefully you wouldn't
> have write-only attributes, and then NLA_REJECT is basically equivalent?
>
If ACL says "-w-" then reading should get explicit permission denied
code possibly with an extack which is more descriptive that reading
is not allowed.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Johannes Berg @ 2018-09-18 12:57 UTC (permalink / raw)
To: Jamal Hadi Salim, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <26dd9a66-9515-93aa-e21f-51c37db6be2c@mojatatu.com>
On Tue, 2018-09-18 at 08:55 -0400, Jamal Hadi Salim wrote:
> Execute permission kind of thing? i.e if i understood you correctly
> if acl is "rwx" then attribute can only be written to (or read from) if
> the "thing executing" is complete
But it's not an attribute that you're executing, it's some kind of
command, and then you get the return value of that command in that
attribute?
Say you want to scan for wifi networks - you trigger a scan, later you
get a notification giving you some data about the scan (let's say the
time it took) - there's no way you can set that time attribute.
(NB: it doesn't work this way, we don't have that attribute now, but I
didn't want to pick a more complicated example)
> > What would the practical difference be though? Hopefully you wouldn't
> > have write-only attributes, and then NLA_REJECT is basically equivalent?
> >
>
> If ACL says "-w-" then reading should get explicit permission denied
> code possibly with an extack which is more descriptive that reading
> is not allowed.
Perhaps. But NLA_REJECT comes with an extack string to tell you, so ...
I dunno. I think we already bloated the policies too much by including
the validation_data pointer, and would hate to add more to that :-)
johannes
^ permalink raw reply
* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Andrew Lunn @ 2018-09-18 13:02 UTC (permalink / raw)
To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <20180918105817.z2o5yybcth7diqsu@verge.net.au>
> Hi Andrew,
Hi Simon
Thanks for the dumps
> 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
>
> basic status: no link
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD flow-control
> link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>
> 2. net-next with this patch reverted
>
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD
> link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
So flow-control is not present here.
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD
> link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
And here also.
Looking at the code, i see:
/* E-MAC init function */
static void ravb_emac_init(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
/* Receive frame limit set register */
ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
/* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
(ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
ECMR_TE | ECMR_RE, ECMR);
Does this mean Pause is not supported in the hardware?
Thanks
Andrew
^ permalink raw reply
* [RFC 1/5] netlink: remove NLA_NESTED_COMPAT
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
This isn't used anywhere, so we might as well get rid of it.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/netlink.h | 2 --
lib/nlattr.c | 11 -----------
2 files changed, 13 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 318b1ded3833..b680fe365e91 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -172,7 +172,6 @@ enum {
NLA_FLAG,
NLA_MSECS,
NLA_NESTED,
- NLA_NESTED_COMPAT,
NLA_NUL_STRING,
NLA_BINARY,
NLA_S8,
@@ -203,7 +202,6 @@ enum {
* NLA_BINARY Maximum length of attribute payload
* NLA_NESTED Don't use `len' field -- length verification is
* done by checking len of nested header (or empty)
- * NLA_NESTED_COMPAT Minimum length of structure payload
* NLA_U8, NLA_U16,
* NLA_U32, NLA_U64,
* NLA_S8, NLA_S16,
diff --git a/lib/nlattr.c b/lib/nlattr.c
index bb6fe5ed4ecf..120ad569e13d 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -140,17 +140,6 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return -ERANGE;
break;
- case NLA_NESTED_COMPAT:
- if (attrlen < pt->len)
- return -ERANGE;
- if (attrlen < NLA_ALIGN(pt->len))
- break;
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN)
- return -ERANGE;
- nla = nla_data(nla) + NLA_ALIGN(pt->len);
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN + nla_len(nla))
- return -ERANGE;
- break;
case NLA_NESTED:
/* a nested attributes is allowed to be empty; if its not,
* it must have a size of at least NLA_HDRLEN.
--
2.14.4
^ permalink raw reply related
* [RFC 2/5] netlink: set extack error message in nla_validate()
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
In nla_parse() we already set this, but it makes sense to
also do it in nla_validate() which already also sets the
bad attribute pointer.
CC: David Ahern <dsahern@gmail.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 120ad569e13d..efbd6c1aff29 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -181,9 +181,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
int rem;
nla_for_each_attr(nla, head, len, rem) {
- int err = validate_nla(nla, maxtype, policy, NULL);
+ static const char _msg[] = "Attribute failed policy validation";
+ const char *msg = _msg;
+ int err = validate_nla(nla, maxtype, policy, &msg);
if (err < 0) {
+ if (extack)
+ extack->_msg = msg;
NL_SET_BAD_ATTR(extack, nla);
return err;
}
--
2.14.4
^ permalink raw reply related
* [RFC 3/5] netlink: combine validate/parse functions
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
The parse function of course contains validate, but it's
implemented a second time, sharing just the validation
of a single attribute.
Introduce nla_validate_parse() that can be used for both
parsing/validation and only validation, to share code.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 76 +++++++++++++++++++++++++++++++-----------------------------
1 file changed, 39 insertions(+), 37 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index efbd6c1aff29..46a6d79cf2d1 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -159,6 +159,37 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return 0;
}
+static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack,
+ struct nlattr **tb)
+{
+ const struct nlattr *nla;
+ int rem;
+
+ nla_for_each_attr(nla, head, len, rem) {
+ static const char _msg[] = "Attribute failed policy validation";
+ const char *msg = _msg;
+ u16 type = nla_type(nla);
+
+ if (policy) {
+ int err = validate_nla(nla, maxtype, policy, &msg);
+
+ if (err < 0) {
+ if (extack)
+ extack->_msg = msg;
+ NL_SET_BAD_ATTR(extack, nla);
+ return err;
+ }
+ }
+
+ if (tb && type > 0 && type <= maxtype)
+ tb[type] = (struct nlattr *)nla;
+ }
+
+ return rem;
+}
+
/**
* nla_validate - Validate a stream of attributes
* @head: head of attribute stream
@@ -177,21 +208,12 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
- const struct nlattr *nla;
int rem;
- nla_for_each_attr(nla, head, len, rem) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
- int err = validate_nla(nla, maxtype, policy, &msg);
+ rem = nla_validate_parse(head, len, maxtype, policy, extack, NULL);
- if (err < 0) {
- if (extack)
- extack->_msg = msg;
- NL_SET_BAD_ATTR(extack, nla);
- return err;
- }
- }
+ if (rem < 0)
+ return rem;
return 0;
}
@@ -245,39 +267,19 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int len, const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
- const struct nlattr *nla;
- int rem, err;
+ int rem;
memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
- nla_for_each_attr(nla, head, len, rem) {
- u16 type = nla_type(nla);
-
- if (type > 0 && type <= maxtype) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
-
- if (policy) {
- err = validate_nla(nla, maxtype, policy, &msg);
- if (err < 0) {
- NL_SET_BAD_ATTR(extack, nla);
- if (extack)
- extack->_msg = msg;
- goto errout;
- }
- }
-
- tb[type] = (struct nlattr *)nla;
- }
- }
+ rem = nla_validate_parse(head, len, maxtype, policy, extack, tb);
+ if (rem < 0)
+ return rem;
if (unlikely(rem > 0))
pr_warn_ratelimited("netlink: %d bytes leftover after parsing attributes in process `%s'.\n",
rem, current->comm);
- err = 0;
-errout:
- return err;
+ return 0;
}
EXPORT_SYMBOL(nla_parse);
--
2.14.4
^ permalink raw reply related
* [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
In one of my previous patches in this area I introduced code
to pass out just the error message to store in the extack, for
use in NLA_REJECT.
Change this code now to set both the error message and the bad
attribute pointer, and carry around a boolean indicating that
the values have been set.
This will be used in the next patch to allow recursive validation
of nested policies, while preserving the innermost error message
rather than overwriting it with a generic out-level message.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 46a6d79cf2d1..fecc7b834706 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -70,7 +70,7 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
static int validate_nla(const struct nlattr *nla, int maxtype,
const struct nla_policy *policy,
- const char **error_msg)
+ struct netlink_ext_ack *extack, bool *extack_set)
{
const struct nla_policy *pt;
int minlen = 0, attrlen = nla_len(nla), type = nla_type(nla);
@@ -95,8 +95,11 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
break;
case NLA_REJECT:
- if (pt->validation_data && error_msg)
- *error_msg = pt->validation_data;
+ if (pt->validation_data && extack && !*extack_set) {
+ *extack_set = true;
+ extack->_msg = pt->validation_data;
+ NL_SET_BAD_ATTR(extack, nla);
+ }
return -EINVAL;
case NLA_FLAG:
@@ -161,24 +164,25 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
- struct netlink_ext_ack *extack,
+ struct netlink_ext_ack *extack, bool *extack_set,
struct nlattr **tb)
{
const struct nlattr *nla;
int rem;
nla_for_each_attr(nla, head, len, rem) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
u16 type = nla_type(nla);
if (policy) {
- int err = validate_nla(nla, maxtype, policy, &msg);
+ int err = validate_nla(nla, maxtype, policy,
+ extack, extack_set);
if (err < 0) {
- if (extack)
- extack->_msg = msg;
- NL_SET_BAD_ATTR(extack, nla);
+ if (!*extack_set) {
+ *extack_set = true;
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Attribute failed policy validation");
+ }
return err;
}
}
@@ -208,9 +212,11 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
+ bool extack_set = false;
int rem;
- rem = nla_validate_parse(head, len, maxtype, policy, extack, NULL);
+ rem = nla_validate_parse(head, len, maxtype, policy,
+ extack, &extack_set, NULL);
if (rem < 0)
return rem;
@@ -267,11 +273,13 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int len, const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
+ bool extack_set = false;
int rem;
memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
- rem = nla_validate_parse(head, len, maxtype, policy, extack, tb);
+ rem = nla_validate_parse(head, len, maxtype, policy,
+ extack, &extack_set, tb);
if (rem < 0)
return rem;
--
2.14.4
^ permalink raw reply related
* [RFC 5/5] netlink: allow NLA_NESTED to specify nested policy to validate
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
Now that we have a validation_data pointer, and the len field in
the policy is unused for NLA_NESTED, we can allow using them both
to have nested validation. This can be nice in code, although we
still have to use nla_parse_nested() or similar which would also
take a policy; however, it also serves as documentation in the
policy without requiring a look at the code.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/netlink.h | 10 ++++++++--
lib/nlattr.c | 17 +++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index b680fe365e91..6efa25a004f5 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -200,8 +200,10 @@ enum {
* NLA_NUL_STRING Maximum length of string (excluding NUL)
* NLA_FLAG Unused
* NLA_BINARY Maximum length of attribute payload
- * NLA_NESTED Don't use `len' field -- length verification is
- * done by checking len of nested header (or empty)
+ * NLA_NESTED Length verification is done by checking len of
+ * nested header (or empty); len field is used if
+ * validation_data is also used, for the max attr
+ * number in the nested policy.
* NLA_U8, NLA_U16,
* NLA_U32, NLA_U64,
* NLA_S8, NLA_S16,
@@ -224,6 +226,10 @@ enum {
* NLA_REJECT This attribute is always rejected and validation data
* may point to a string to report as the error instead
* of the generic one in extended ACK.
+ * NLA_NESTED Points to a nested policy to validate, must also set
+ * `len' to the max attribute number.
+ * Note that nla_parse() will validate, but of course not
+ * parse, the nested sub-policies.
* All other Unused
*
* Example:
diff --git a/lib/nlattr.c b/lib/nlattr.c
index fecc7b834706..4c8c4fffb20d 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -68,6 +68,11 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
return 0;
}
+static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack, bool *extack_set,
+ struct nlattr **tb);
+
static int validate_nla(const struct nlattr *nla, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack, bool *extack_set)
@@ -149,6 +154,18 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
*/
if (attrlen == 0)
break;
+ if (attrlen < NLA_HDRLEN)
+ return -ERANGE;
+ if (pt->validation_data) {
+ int err;
+
+ err = nla_validate_parse(nla_data(nla), nla_len(nla),
+ pt->len, pt->validation_data,
+ extack, extack_set, NULL);
+ if (err < 0)
+ return err;
+ }
+ break;
default:
if (pt->len)
minlen = pt->len;
--
2.14.4
^ permalink raw reply related
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Jamal Hadi Salim @ 2018-09-18 13:12 UTC (permalink / raw)
To: Johannes Berg, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <1537275441.2957.26.camel@sipsolutions.net>
On 2018-09-18 8:57 a.m., Johannes Berg wrote:
> On Tue, 2018-09-18 at 08:55 -0400, Jamal Hadi Salim wrote:
>
>> Execute permission kind of thing? i.e if i understood you correctly
>> if acl is "rwx" then attribute can only be written to (or read from) if
>> the "thing executing" is complete
>
> But it's not an attribute that you're executing, it's some kind of
> command, and then you get the return value of that command in that
> attribute?
>
> Say you want to scan for wifi networks - you trigger a scan, later you
> get a notification giving you some data about the scan (let's say the
> time it took) - there's no way you can set that time attribute.
>
Not very familiar with how wifi scan gets initiated. I am guessing
you issue some GET or SET to start a scan - and you get an async
response when it is complete (and it would include the time it took)?
Or maybe you get an immediate response and event notification later
and the time spent is in that notification?
I would still see that as a read-only attribute.
And the utility of "execute" bit is only in blocking another scan
from being initiated when one is in progress, if that is a desired
goal.
Note in most net devices stats can only be read but not written to
for example.
> (NB: it doesn't work this way, we don't have that attribute now, but I
> didn't want to pick a more complicated example)
>
>>> What would the practical difference be though? Hopefully you wouldn't
>>> have write-only attributes, and then NLA_REJECT is basically equivalent?
>>>
>>
>> If ACL says "-w-" then reading should get explicit permission denied
>> code possibly with an extack which is more descriptive that reading
>> is not allowed.
>
> Perhaps. But NLA_REJECT comes with an extack string to tell you, so ...
>
> I dunno. I think we already bloated the policies too much by including
> the validation_data pointer, and would hate to add more to that :-)
Your mileage may vary. NLA_REJECT may work acls offer more fine grained
controls.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] netlink: ipv4 igmp join notifications
From: Patrick Ruddy @ 2018-09-18 13:12 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, Jiří Pírko, Stephen Hemminger,
Nikolay Aleksandrov
In-Reply-To: <CAJieiUg03p6MNkZCdO65p4uQuOCVTN4mDr0htQNeYumrpDQMeA@mail.gmail.com>
On Thu, 2018-09-13 at 10:03 -0700, Roopa Prabhu wrote:
> On Thu, Sep 6, 2018 at 8:40 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> > On Thu, Sep 6, 2018 at 2:10 AM, Patrick Ruddy
> > <pruddy@vyatta.att-mail.com> wrote:
> > > Some userspace applications need to know about IGMP joins from the
> > > kernel for 2 reasons:
> > > 1. To allow the programming of multicast MAC filters in hardware
> > > 2. To form a multicast FORUS list for non link-local multicast
> > > groups to be sent to the kernel and from there to the interested
> > > party.
> > > (1) can be fulfilled but simply sending the hardware multicast MAC
> > > address to be programmed but (2) requires the L3 address to be sent
> > > since this cannot be constructed from the MAC address whereas the
> > > reverse translation is a standard library function.
> > >
> > > This commit provides addition and deletion of multicast addresses
> > > using the RTM_NEWMDB and RTM_DELMDB messages with AF_INET. It also
> > > provides the RTM_GETMDB extension to allow multicast join state to
> > > be read from the kernel.
> > >
> > > Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com>
> > > ---
> > > v3 rework to use RTM_***MDB messages as per review comments.
> >
> > Patrick, this version seems to be using RTM_***MDB msgs with the
> > RTM_*ADDR format.
> > We cant do that...because existing RTM_MDB users will be confused.
> >
> > My request was to evaluate RTM_***MDB msg format. see
> > nlmsg_populate_mdb_fill for details.
> >
> > If you can wait a day or two I can share some experimental code that
> > moves high level RTM_*MDB msg handling into net/core/rtnetlink.c
> > similar to RTM_*FDB
> >
>
> I was trying to get a default per interface (non bridge) RTM_*MDB
> working, but realized that the dev->mc
> entries are already getting dumped as part of RTM_*FDB msgs instead of
> RTM_*MDB. (see net/core/rtnetlink.c:ndo_dflt_fdb_dump).
> This adds another wrench.
>
> so, that puts us back to your use of RTM_NEWADDR.
> Instead of using IFA_ADDRESS, you could introduce a new one
> IFA_IGMP_MULTICAST (since IFA_MULTICAST is already taken).
>
>
> To keep existing users of RTM_NEWADDR unaffected. I think you can use
> the IPMR family with RTM_NEWADDR.
> We can introduce new notification group. (We can choose to add a new
> family too, but that seems unnecessary)
>
> since you only need dumps:
> rtnl_register(RTNL_FAMILY_IPMR, RTM_GETADDR, NULL, igmp_rtm_dumpaddrs, 0);
>
> For notifications, since we already have many variants for routes, I
> don't see a problem adding similar addr variants
> RTNLGRP_IPV4_MCADDR
>
> (Others on the list may have more feedback).
I've hit a small snag with adding the new groups. The number of defined
groups currently sits at 31 so I can only add one before hitting the
limit defined by the 32 bit groups bitmask in socakddr_nl. I can use 1
group for both v4 and v6 notifications which seems like the sensible
options since the AF is carried separately, but it breaks the precedent
where there are separate IPV4 and IPV6 groups for IFADDR.
I have the combined group patches ready and can share them if that's
the preference.
Has there been any previous discussion about extending the number of
availabel groups?
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Ard Biesheuvel @ 2018-09-18 18:53 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andrew Lutomirski, David Miller, Andrew Lunn, Eric Biggers,
Greg Kroah-Hartman, LKML, Netdev, Samuel Neves,
Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAHmME9qwRzuoo-3Hxahwu=Li2LCz06Uowaq1GFmkts6tsffM7w@mail.gmail.com>
On 17 September 2018 at 08:52, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi Ard,
>
> On Mon, Sep 17, 2018 at 7:26 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> OK, so let me summarize my remaining concerns as well. I may be a bit
>> more finicky than Andy, though.
>
> Yes, and generally hostile to this whole initiative since the
> beginning. But I am very grateful for your reviews nonetheless, and
> I'll do my best to incorporate as much as is reasonable.
>
>> I would like to urge Jason to
>> bear with us and bring this discussion to a close before resubmitting.
>
> What I fear is that either:
> - You don't like the Zinc initiative in one way or another, and the
> desire to "keep discussing" and adding more things is less out of
> their necessity and more out of a desire to stall it indefinitely.
> - You're going to bikeshed and bikeshed and waste tons of time until
> Zinc copies lots of the same design decisions from the present crypto
> API.
>
Given that you show no interest whatsoever in gaining an understanding
of the underlying requirements that we have to deal with in the crypto
API, the only way to get my point across is by repeatedly stating it
in response to your patches. Also, sending out a new series each time
with only half of the review comments addressed doesn't make me a
bikeshedder.
> I do sincerely hope these are only fears and not what actually is
> going on. I'll do my best to take into serious consideration what you
> say -- many are indeed extremely helpful -- but I certainly won't be
> wholesale accepting all the things you've mentioned.
>
I have pointed out to you numerous times (as has Eric) that the
ChaCha20 ARM code you are importing from the OpenSSL project has been
found to be slower on Cortex-A7, which represents the vast majority of
devices expected to be in the field in 1~2 years. Yet, we are at the
fifth revision now where you are replacing the existing code. Also, I
have asked you more than once to split out your changes to the
upstream OpenSSL code into separate patches so we can more easily
track them, but v5 now puts them in the commit log (again) [but in a
corrupted way - the preprocessor directives are filtered out by
git-commit], which means we cannot use git diff/blame etc to look at
them.
Upstreaming code is about taking an interest in other people's use
cases, and about choosing your battles. It is unfortunate that we have
spent all this time talking about a couple of crypto routines, while
the actual meat of your submission is in WireGuard itself, which I'm
sure you much rather talk about.
>> * simd_relax() is currently not called by the crypto routines
>> themselves. This means that the worst case scheduling latency is
>> unbounded, which is unacceptable for the -rt kernel. The worst case
>> scheduling latency should never be proportional to the input size.
>> (Apologies for not spotting that earlier)
>
> Good catch. I actually did this correct when porting the crypto API to
> use Zinc (in those later top commits in v4), but I hadn't in the Zinc
> code itself. I'll address this for v5.
>
>> maintainership
>> responsibilities
>
> Samuel and I intend to maintain Zinc in lib/zinc/ and send future
> updates to it through Greg's tree, as mentioned in the 00/ cover
> letter. The maintainership of the existing crypto API won't change.
>
>> * The current organization of the code puts all available (for the
>> arch) versions of all routines into a single module, which can only be
>> built in once we update random.c to use Zinc's chacha20 routines. This
>> bloats the core kernel (which is a huge deal for embedded systems that
>> have very strict boot requirements).
>
> I'll split each Zinc primitive into a separate module for v5, per your
> and Andy's desire. And the SIMD code is already toggle-able via a
> Kconfig menu option.
>
>> we should
>> work with Andy Polyakov (as I have done several times over the past 5+
>> years) to upstream the changes we apply to the kernel version of the
>> code.
>
> Indeed this is the intent.
>
>> The same applies to code from other sources, btw, but I am not
>> personally familiar with them.
>
> Good news on this front:
> - Rene wrote the MIPS code specifically for WireGuard, so that _is_ upstream.
> - Samuel wrote the BLAKE2 assembly, and he's the co-maintainer of Zinc with me.
> - I talk to Dan and Peter a decent amount about qhasm.
> - I'm in very close contact with the team behind HACL*, and they're
> treating Zinc as a target -- stylistically and with regards to kernel
> requirements -- which means they're looking at what's happening in
> this patchset and adjusting accordingly.
>
>
>> * If upstreaming the changes is not an option, they should be applied
>> as a separate patch and not hidden in a 5000 line patch without any
>> justification or documentation (but Jason is already working on that)
>
> Indeed this is already underway.
>
> Thanks again for your review.
>
> Jason
^ permalink raw reply
* Re: [PATCH]ipv6: multicast: In mld_send_cr function moving read lock to second for loop
From: Guruswamy Basavaiah @ 2018-09-18 13:21 UTC (permalink / raw)
To: davem; +Cc: netdev, Alexey Kuznetsov, Hideaki YOSHIFUJI
In-Reply-To: <20180818.135819.1279366350823380240.davem@davemloft.net>
> This will lead to deadlocks, idev->mc_lock must be taken with _bh().
Modified the existing spin_lock to spin_lock_bh
> I have zero confidence in this change, did you do any stress testing
> with lockdep enabled? It would have caught this quickly.
With LOCKDEP enabled ran LTP multicast stress with the below new patch.
Test case is successful and LOCKDEP did not catch any dead lock issues.
---
>From 789840a6c6f783311ea7dfd832787c27d5b8359f Mon Sep 17 00:00:00 2001
From: Guruswamy Basavaiah <guru2018@gmail.com>
Date: Tue, 18 Sep 2018 18:40:21 +0530
Subject: [PATCH] ipv6: multicast: In mld_send_cr function moving read lock to
second for loop
In function mld_send_cr, the first loop is already protected by
idev->mc_lock, it dont need idev->lock read lock, hence moving it
only to second for loop. And converting the existing spin_lock to
spin_lock_bh
Signed-off-by: Guruswamy Basavaiah <guru2018@gmail.com>
---
net/ipv6/mcast.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 4ae54aaca373..751e580eb0ed 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1912,8 +1912,7 @@ static void mld_send_cr(struct inet6_dev *idev)
struct sk_buff *skb = NULL;
int type, dtype;
- read_lock_bh(&idev->lock);
- spin_lock(&idev->mc_lock);
+ spin_lock_bh(&idev->mc_lock);
/* deleted MCA's */
pmc_prev = NULL;
@@ -1947,8 +1946,9 @@ static void mld_send_cr(struct inet6_dev *idev)
} else
pmc_prev = pmc;
}
- spin_unlock(&idev->mc_lock);
+ spin_unlock_bh(&idev->mc_lock);
+ read_lock_bh(&idev->lock);
/* change recs */
for (pmc = idev->mc_list; pmc; pmc = pmc->next) {
spin_lock_bh(&pmc->mca_lock);
--
2.14.4
On Sun, 19 Aug 2018 at 02:28, David Miller <davem@davemloft.net> wrote:
>
> From: Guruswamy Basavaiah <guru2018@gmail.com>
> Date: Fri, 17 Aug 2018 18:01:41 +0530
>
> > @@ -1860,7 +1860,6 @@ static void mld_send_cr(struct inet6_dev *idev)
> > struct sk_buff *skb = NULL;
> > int type, dtype;
> >
> > - read_lock_bh(&idev->lock);
> > spin_lock(&idev->mc_lock);
> >
> > /* deleted MCA's */
>
> This will lead to deadlocks, idev->mc_lock must be taken with _bh().
>
> I have zero confidence in this change, did you do any stress testing
> with lockdep enabled? It would have caught this quickly.
--
Guruswamy Basavaiah
^ permalink raw reply related
* Re: pegged softirq and NAPI race (?)
From: Eric Dumazet @ 2018-09-18 13:45 UTC (permalink / raw)
To: songliubraving
Cc: netdev, Jeff Kirsher, Alexander Duyck, michael.chan, kernel-team
In-Reply-To: <A782704A-DF97-4E85-B10A-D2268A67DFD7@fb.com>
On Tue, Sep 18, 2018 at 1:41 AM Song Liu <songliubraving@fb.com> wrote:
>
> We are debugging this issue that netconsole message triggers pegged softirq
> (ksoftirqd taking 100% CPU for many seconds). We found this issue in
> production with both bnxt and ixgbe, on a 4.11 based kernel. This is easily
> reproducible with ixgbe on 4.11, and latest net/net-next (see [1] for more
> detail).
>
> After debugging for some time, we found that this issue is likely related
> to 39e6c8208d7b ("net: solve a NAPI race"). After reverting this commit,
> the steps described in [1] cannot reproduce the issue on ixgbe. Reverting
> this commit also reduces the chances we hit the issue with bnxt (it still
> happens with a lower rate).
>
> I tried to fix this issue with relaxed variant (or older version) of
> napi_schedule_prep() in netpoll, just like the one on napi_watchdog().
> However, my tests do not always go as expected.
>
> Please share your comments/suggestions on which direction shall we try
> to fix this.
>
> Thanks in advance!
> Song
>
>
> [1] https://www.spinics.net/lists/netdev/msg522328.html
You have not traced ixgbe to understand why driver hits
"clean_complete=false" all the time ?
^ permalink raw reply
* [PATCH net 0/5] net/smc: fixes 2018-09-18
From: Ursula Braun @ 2018-09-18 13:46 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
Dave,
here are some fixes in different areas of the smc code for the net
tree.
Thanks, Ursula
Karsten Graul (1):
net/smc: no urgent data check for listen sockets
Ursula Braun (3):
net/smc: fix non-blocking connect problem
net/smc: remove duplicate mutex_unlock
net/smc: enable fallback for connection abort in state INIT
YueHaibing (1):
net/smc: fix sizeof to int comparison
net/smc/af_smc.c | 26 ++++++++++++++++----------
net/smc/smc_clc.c | 14 ++++++--------
net/smc/smc_close.c | 14 +++++++-------
3 files changed, 29 insertions(+), 25 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH net 1/5] net/smc: fix non-blocking connect problem
From: Ursula Braun @ 2018-09-18 13:46 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180918134638.90271-1-ubraun@linux.ibm.com>
In state SMC_INIT smc_poll() delegates polling to the internal
CLC socket. This means, once the connect worker has finished
its kernel_connect() step, the poll wake-up may occur. This is not
intended. The wake-up should occur from the wake up call in
smc_connect_work() after __smc_connect() has finished.
Thus in state SMC_INIT this patch now calls sock_poll_wait() on the
main SMC socket.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 2d8a1e15e4f9..9c3976bcde46 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -742,7 +742,10 @@ static void smc_connect_work(struct work_struct *work)
smc->sk.sk_err = -rc;
out:
- smc->sk.sk_state_change(&smc->sk);
+ if (smc->sk.sk_err)
+ smc->sk.sk_state_change(&smc->sk);
+ else
+ smc->sk.sk_write_space(&smc->sk);
kfree(smc->connect_info);
smc->connect_info = NULL;
release_sock(&smc->sk);
@@ -1529,7 +1532,7 @@ static __poll_t smc_poll(struct file *file, struct socket *sock,
return EPOLLNVAL;
smc = smc_sk(sock->sk);
- if ((sk->sk_state == SMC_INIT) || smc->use_fallback) {
+ if (smc->use_fallback) {
/* delegate to CLC child sock */
mask = smc->clcsock->ops->poll(file, smc->clcsock, wait);
sk->sk_err = smc->clcsock->sk->sk_err;
--
2.16.4
^ permalink raw reply related
* [PATCH net 2/5] net/smc: remove duplicate mutex_unlock
From: Ursula Braun @ 2018-09-18 13:46 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180918134638.90271-1-ubraun@linux.ibm.com>
For a failing smc_listen_rdma_finish() smc_listen_decline() is
called. If fallback is possible, the new socket is already enqueued
to be accepted in smc_listen_decline(). Avoid enqueuing a second time
afterwards in this case, otherwise the smc_create_lgr_pending lock
is released twice:
[ 373.463976] WARNING: bad unlock balance detected!
[ 373.463978] 4.18.0-rc7+ #123 Tainted: G O
[ 373.463979] -------------------------------------
[ 373.463980] kworker/1:1/30 is trying to release lock (smc_create_lgr_pending) at:
[ 373.463990] [<000003ff801205fc>] smc_listen_work+0x22c/0x5d0 [smc]
[ 373.463991] but there are no more locks to release!
[ 373.463991]
other info that might help us debug this:
[ 373.463993] 2 locks held by kworker/1:1/30:
[ 373.463994] #0: 00000000772cbaed ((wq_completion)"events"){+.+.}, at: process_one_work+0x1ec/0x6b0
[ 373.464000] #1: 000000003ad0894a ((work_completion)(&new_smc->smc_listen_work)){+.+.}, at: process_one_work+0x1ec/0x6b0
[ 373.464003]
stack backtrace:
[ 373.464005] CPU: 1 PID: 30 Comm: kworker/1:1 Kdump: loaded Tainted: G O 4.18.0-rc7uschi+ #123
[ 373.464007] Hardware name: IBM 2827 H43 738 (LPAR)
[ 373.464010] Workqueue: events smc_listen_work [smc]
[ 373.464011] Call Trace:
[ 373.464015] ([<0000000000114100>] show_stack+0x60/0xd8)
[ 373.464019] [<0000000000a8c9bc>] dump_stack+0x9c/0xd8
[ 373.464021] [<00000000001dcaf8>] print_unlock_imbalance_bug+0xf8/0x108
[ 373.464022] [<00000000001e045c>] lock_release+0x114/0x4f8
[ 373.464025] [<0000000000aa87fa>] __mutex_unlock_slowpath+0x4a/0x300
[ 373.464027] [<000003ff801205fc>] smc_listen_work+0x22c/0x5d0 [smc]
[ 373.464029] [<0000000000197a68>] process_one_work+0x2a8/0x6b0
[ 373.464030] [<0000000000197ec2>] worker_thread+0x52/0x410
[ 373.464033] [<000000000019fd0e>] kthread+0x15e/0x178
[ 373.464035] [<0000000000aaf58a>] kernel_thread_starter+0x6/0xc
[ 373.464052] [<0000000000aaf584>] kernel_thread_starter+0x0/0xc
[ 373.464054] INFO: lockdep is turned off.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 9c3976bcde46..5c6d30eb4a71 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1153,9 +1153,9 @@ static int smc_listen_rdma_reg(struct smc_sock *new_smc, int local_contact)
}
/* listen worker: finish RDMA setup */
-static void smc_listen_rdma_finish(struct smc_sock *new_smc,
- struct smc_clc_msg_accept_confirm *cclc,
- int local_contact)
+static int smc_listen_rdma_finish(struct smc_sock *new_smc,
+ struct smc_clc_msg_accept_confirm *cclc,
+ int local_contact)
{
struct smc_link *link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];
int reason_code = 0;
@@ -1178,11 +1178,12 @@ static void smc_listen_rdma_finish(struct smc_sock *new_smc,
if (reason_code)
goto decline;
}
- return;
+ return 0;
decline:
mutex_unlock(&smc_create_lgr_pending);
smc_listen_decline(new_smc, reason_code, local_contact);
+ return reason_code;
}
/* setup for RDMA connection of server */
@@ -1279,8 +1280,10 @@ static void smc_listen_work(struct work_struct *work)
}
/* finish worker */
- if (!ism_supported)
- smc_listen_rdma_finish(new_smc, &cclc, local_contact);
+ if (!ism_supported) {
+ if (smc_listen_rdma_finish(new_smc, &cclc, local_contact))
+ return;
+ }
smc_conn_save_peer_info(new_smc, &cclc);
mutex_unlock(&smc_create_lgr_pending);
smc_listen_out_connected(new_smc);
--
2.16.4
^ permalink raw reply related
* [PATCH net 3/5] net/smc: enable fallback for connection abort in state INIT
From: Ursula Braun @ 2018-09-18 13:46 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180918134638.90271-1-ubraun@linux.ibm.com>
If a linkgroup is terminated abnormally already due to failing
LLC CONFIRM LINK or LLC ADD LINK, fallback to TCP is still possible.
In this case do not switch to state SMC_PEERABORTWAIT and do not set
sk_err.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_close.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index ac961dfb1ea1..ea2b87f29469 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -100,15 +100,14 @@ static void smc_close_active_abort(struct smc_sock *smc)
struct smc_cdc_conn_state_flags *txflags =
&smc->conn.local_tx_ctrl.conn_state_flags;
- sk->sk_err = ECONNABORTED;
- if (smc->clcsock && smc->clcsock->sk) {
- smc->clcsock->sk->sk_err = ECONNABORTED;
- smc->clcsock->sk->sk_state_change(smc->clcsock->sk);
+ if (sk->sk_state != SMC_INIT && smc->clcsock && smc->clcsock->sk) {
+ sk->sk_err = ECONNABORTED;
+ if (smc->clcsock && smc->clcsock->sk) {
+ smc->clcsock->sk->sk_err = ECONNABORTED;
+ smc->clcsock->sk->sk_state_change(smc->clcsock->sk);
+ }
}
switch (sk->sk_state) {
- case SMC_INIT:
- sk->sk_state = SMC_PEERABORTWAIT;
- break;
case SMC_ACTIVE:
sk->sk_state = SMC_PEERABORTWAIT;
release_sock(sk);
@@ -143,6 +142,7 @@ static void smc_close_active_abort(struct smc_sock *smc)
case SMC_PEERFINCLOSEWAIT:
sock_put(sk); /* passive closing */
break;
+ case SMC_INIT:
case SMC_PEERABORTWAIT:
case SMC_CLOSED:
break;
--
2.16.4
^ permalink raw reply related
* Re: [PATCH][net-next] netlink: avoid to allocate full skb when sending to many devices
From: Eric Dumazet @ 2018-09-18 14:08 UTC (permalink / raw)
To: Li RongQing, netdev
In-Reply-To: <1537248415-15534-1-git-send-email-lirongqing@baidu.com>
On 09/17/2018 10:26 PM, Li RongQing wrote:
> if skb->head is vmalloc address, when this skb is delivered, full
> allocation for this skb is required, if there are many devices,
> the full allocation will be called for every devices
>
> now using the first time allocated skb when iterate other devices
> to send, reduce full allocation and speedup deliver.
>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> net/netlink/af_netlink.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
>
This looks very broken to me.
Only the original skb (given as an argument to __netlink_deliver_tap()) is guaranteed to not
disappear while the loop is performed.
(There is no skb_clone() after the first netlink_to_full_skb())
^ permalink raw reply
* [PATCH 4/4] media: bpf: add bpf function to report mouse movement
From: Sean Young @ 2018-09-18 14:29 UTC (permalink / raw)
To: linux-media; +Cc: netdev
In-Reply-To: <20180918142930.6686-1-sean@mess.org>
Some IR remotes have a directional pad or other pointer-like thing that
can be used as a mouse. Make it possible to decode these types of IR
protocols in BPF.
Cc: netdev@vger.kernel.org
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/rc/bpf-lirc.c | 24 +++++++++++++++++++++++
include/uapi/linux/bpf.h | 17 +++++++++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 2 ++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c
index 8b97fd1f0cea..390a722e6211 100644
--- a/drivers/media/rc/bpf-lirc.c
+++ b/drivers/media/rc/bpf-lirc.c
@@ -59,6 +59,28 @@ static const struct bpf_func_proto rc_keydown_proto = {
.arg4_type = ARG_ANYTHING,
};
+BPF_CALL_3(bpf_rc_pointer_rel, u32*, sample, s32, rel_x, s32, rel_y)
+{
+ struct ir_raw_event_ctrl *ctrl;
+
+ ctrl = container_of(sample, struct ir_raw_event_ctrl, bpf_sample);
+
+ input_report_rel(ctrl->dev->input_dev, REL_X, rel_x);
+ input_report_rel(ctrl->dev->input_dev, REL_Y, rel_y);
+ input_sync(ctrl->dev->input_dev);
+
+ return 0;
+}
+
+static const struct bpf_func_proto rc_pointer_rel_proto = {
+ .func = bpf_rc_pointer_rel,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+};
+
static const struct bpf_func_proto *
lirc_mode2_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
@@ -67,6 +89,8 @@ lirc_mode2_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &rc_repeat_proto;
case BPF_FUNC_rc_keydown:
return &rc_keydown_proto;
+ case BPF_FUNC_rc_pointer_rel:
+ return &rc_pointer_rel_proto;
case BPF_FUNC_map_lookup_elem:
return &bpf_map_lookup_elem_proto;
case BPF_FUNC_map_update_elem:
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 66917a4eba27..20d6750b687c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2141,6 +2141,20 @@ union bpf_attr {
* request in the skb.
* Return
* 0 on success, or a negative error in case of failure.
+ *
+ * int bpf_rc_pointer_rel(void *ctx, s32 rel_x, s32 rel_y)
+ * Description
+ * This helper is used in programs implementing IR decoding, to
+ * report a successfully decoded pointer movement.
+ *
+ * The *ctx* should point to the lirc sample as passed into
+ * the program.
+ *
+ * This helper is only available is the kernel was compiled with
+ * the **CONFIG_BPF_LIRC_MODE2** configuration option set to
+ * "**y**".
+ * Return
+ * 0
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2226,7 +2240,8 @@ union bpf_attr {
FN(get_current_cgroup_id), \
FN(get_local_storage), \
FN(sk_select_reuseport), \
- FN(skb_ancestor_cgroup_id),
+ FN(skb_ancestor_cgroup_id), \
+ FN(rc_pointer_rel),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index e4be7730222d..edda498c1231 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -143,6 +143,8 @@ static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
(void *) BPF_FUNC_skb_cgroup_id;
static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
(void *) BPF_FUNC_skb_ancestor_cgroup_id;
+static int (*bpf_rc_pointer_rel)(void *ctx, int rel_x, int rel_y) =
+ (void *) BPF_FUNC_rc_pointer_rel;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] net: apm: xgene: force XGene enet driver to re-balance IRQ usage
From: Al Stone @ 2018-09-18 20:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, isubramanian, kchudgar, qnguyen
In-Reply-To: <20180917.193531.1424852853827678152.davem@davemloft.net>
On 09/17/2018 08:35 PM, David Miller wrote:
> From: Al Stone <ahs3@redhat.com>
> Date: Mon, 17 Sep 2018 17:35:33 -0600
>
>> @@ -866,8 +866,11 @@ static int xgene_enet_napi(struct napi_struct *napi, const int budget)
>> processed = xgene_enet_process_ring(ring, budget);
>>
>> if (processed != budget) {
>> + struct irq_desc *desc = irq_to_desc(ring->irq);
>> +
>> napi_complete_done(napi, processed);
>> - enable_irq(ring->irq);
>> + if (desc && desc->depth > 0)
>> + enable_irq(ring->irq);
>
> I really can't accept a patch that grovels into IRQ layer internals
> to work around a driver's IRQ enable/disable usage problem.
>
> Sorry.
No worries. I hesitated even sending it, actually. The rewrite of the driver
that is needed is just something no one seems to have any interest in, or any
time for, but it really needs it to fix this properly (a colleague has found at
least one other issue in the structure of this driver, for example). The cost
of doing that rewrite/restructure, though, far outweighs the benefit for this
device -- especially when the workaround is "don't run tuned".
I'll put a proper fix back on the list of Interesting Things to Work On When
Time Allows, for now.
Thanks for the quick -- and kind :) -- response. I really appreciate it.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------
^ permalink raw reply
* Re: linux-next: Tree for Sep 18 (kernel/bpf/syscall)
From: Willem de Bruijn @ 2018-09-18 20:22 UTC (permalink / raw)
To: Daniel Borkmann
Cc: rdunlap, Stephen Rothwell, Linux-Next Mailing List, LKML,
Network Development, Alexei Starovoitov, Petar Penkov,
Willem de Bruijn
In-Reply-To: <CAF=yD-Kv7ig9UEo8Ci3eZOV=spa9cbGA5Mfa5Too8TgUzVA3fA@mail.gmail.com>
On Tue, Sep 18, 2018 at 12:28 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Sep 18, 2018 at 11:44 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > [ Cc Petar and Willem ]
> >
> > On 09/18/2018 05:37 PM, Randy Dunlap wrote:
> > > On 9/17/18 10:12 PM, Stephen Rothwell wrote:
> > >> Hi all,
> > >>
> > >> Changes since 20180913:
> > >
> > > on i386 or x86_64: (in 6 of 20 randconfigs)
> > >
> > > kernel/bpf/syscall.o: In function `__x64_sys_bpf':
> > > syscall.c:(.text+0x3278): undefined reference to `skb_flow_dissector_bpf_prog_attach'
> > > syscall.c:(.text+0x3310): undefined reference to `skb_flow_dissector_bpf_prog_detach'
> > > kernel/bpf/syscall.o:(.rodata+0x3f0): undefined reference to `flow_dissector_prog_ops'
> > > kernel/bpf/verifier.o:(.rodata+0x250): undefined reference to `flow_dissector_verifier_ops'
> > >
> > > Full randconfig file is attached.
> >
> > Looks like we need a wrapper for these, config had:
> >
> > # CONFIG_NET is not set
>
> Thanks for forwarding the report.
>
> For the functions, I think we can use a similar static inline stub as
> used e.g., for bpf_map_offload_map_alloc.
>
> The _ops references are from a macro that includes linux/bpf_types.h,
> so indeed a CONFIG looks needed.
>
> I need to look it over before I send out, but tentative patch:
Sent http://patchwork.ozlabs.org/patch/971280/
^ permalink raw reply
* Re: [PATCH net-next v5 06/20] zinc: ChaCha20 MIPS32r2 implementation
From: Paul Burton @ 2018-09-18 20:25 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-crypto@vger.kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, René van Dorst, Samuel Neves,
Andy Lutomirski, Jean-Philippe Aumasson, Ralf Baechle,
James Hogan, linux-mips@linux-mips.org
In-Reply-To: <20180918161646.19105-7-Jason@zx2c4.com>
Hi Jason,
On Tue, Sep 18, 2018 at 06:16:32PM +0200, Jason A. Donenfeld wrote:
> +.Lchacha20_mips_xor_aligned_4_b:
> + /* STORE_ALIGNED( 4, 0, $sp, 0+CONSTANT_OFS_SP) */
> + lw T0, 0+CONSTANT_OFS_SP($sp)
> + lw T1, 0(IN)
> + addu X0, T0
> + CPU_TO_LE32(X0)
> + xor X0, T1
> + .set noreorder
> + bne OUT, PTR_LAST_ROUND, .Loop_chacha20_rounds
> + sw X0, 0(OUT)
> + .set reorder
> +
> + .set noreorder
> + bne $at, BYTES, .Lchacha20_mips_xor_bytes
> + /* Empty delayslot, Increase NONCE_0, return NONCE_0 value */
> + addiu NONCE_0, 1
> + .set noreorder
Should this be .set reorder?
Even better - could we not just place the addiu before the bne & drop
the .set noreorder, allowing the assembler to fill the delay slot with
the addiu? Likewise in many other places throughout the patch.
That would be more future proof - particularly if we ever want to adjust
this for use with the nanoMIPS ISA which has no delay slots. It may also
allow the assembler the choice to use compact branches (ie. branches
without visible delay slots) when targeting MIPS32r6. I know neither of
these will currently build this code, but I think avoiding all the
noreorder blocks would be a nice cleanup just for the sake of
readability anyway.
Thanks,
Paul
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH net-next v2 02/14] iavf: diet and reformat
From: Bowers, AndrewX @ 2018-09-18 14:57 UTC (permalink / raw)
To: Brandeburg, Jesse, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org
In-Reply-To: <20180915003757.169108-3-jesse.brandeburg@intel.com>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Jesse Brandeburg
> Sent: Friday, September 14, 2018 5:38 PM
> To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v2 02/14] iavf: diet and reformat
>
> Remove a bunch of unused code and reformat a few lines. Also
> remove some now un-necessary files.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> drivers/net/ethernet/intel/iavf/i40e_adminq.c | 27 -
> drivers/net/ethernet/intel/iavf/i40e_adminq_cmd.h | 2277 +------------------
> --
> drivers/net/ethernet/intel/iavf/i40e_common.c | 337 ---
> drivers/net/ethernet/intel/iavf/i40e_hmc.h | 215 --
> drivers/net/ethernet/intel/iavf/i40e_lan_hmc.h | 158 --
> drivers/net/ethernet/intel/iavf/i40e_prototype.h | 65 +-
> drivers/net/ethernet/intel/iavf/i40e_register.h | 245 ---
> drivers/net/ethernet/intel/iavf/i40e_type.h | 783 +------
> 8 files changed, 50 insertions(+), 4057 deletions(-)
> delete mode 100644 drivers/net/ethernet/intel/iavf/i40e_hmc.h
> delete mode 100644 drivers/net/ethernet/intel/iavf/i40e_lan_hmc.h
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-18 20:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andrew Lutomirski, David Miller, Andrew Lunn, Eric Biggers,
Greg Kroah-Hartman, LKML, Netdev, Samuel Neves,
Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAKv+Gu8OKyVvvgLy48GDV2KJt-UmmHbf5x0qNxQC_SDiddb9-g@mail.gmail.com>
Hi Ard,
On Tue, Sep 18, 2018 at 11:53:11AM -0700, Ard Biesheuvel wrote:
> On 17 September 2018 at 08:52, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > Hi Ard,
> >
>
> Given that you show no interest whatsoever in gaining an understanding
> of the underlying requirements that we have to deal with in the crypto
> API, the only way to get my point across is by repeatedly stating it
Sorry if I've come across that way, but I am certainly interested in
gaining such an understanding of said requirements.
> I have pointed out to you numerous times (as has Eric) that the
> ChaCha20 ARM code you are importing from the OpenSSL project has been
> found to be slower on Cortex-A7, which represents the vast majority of
> devices expected to be in the field in 1~2 years.
I mentioned in the other thread that I intend immanently to begin
benching on a variety of ARM boards, and I should have numbers and
results and conclusions somewhat soon for the list.
My initial notion here was that it'd be better to go with AndyP's code
no matter what, and then later work with him on contributing said
improvements, and then port these back to the kernel. However, you and
Andy made a compelling point about code replacement -- that it's okay to
replace all in one go only if there are positive benchmark results. So I
think what I'll do to appease this is -- if the benchmarks are indeed
how Eric suggested -- stick with your faster code, and then follow up
with replacement plans after the merge. (I feel a bit more comfortable
with varying ChaCha code, because implementations tend to be pretty
straight forward and harder to screw up in subtle ways than, say,
poly1305 or curve25519.)
> have asked you more than once to split out your changes to the
> upstream OpenSSL code into separate patches so we can more easily
> track them, but v5 now puts them in the commit log (again) [but in a
> corrupted way - the preprocessor directives are filtered out by
> git-commit], which means we cannot use git diff/blame etc to look at
> them.
Didnt't realize this was so important to you. It's trivial to do, so
I'll do that for AndyP's implementations for the next revision.
> Upstreaming code is about taking an interest in other people's use
> cases, and about choosing your battles. It is unfortunate that we have
> spent all this time talking about a couple of crypto routines, while
> the actual meat of your submission is in WireGuard itself, which I'm
> sure you much rather talk about.
I don't find it unfortunate; getting the crypto right is of the utmost
importance.
Regards,
Jason
^ permalink raw reply
* Re: [PATCH net-next v5 00/20] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-09-18 21:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Greg Kroah-Hartman
In-Reply-To: <CAKv+Gu8doWS+BYzqoT7499CJX0hLm1jBSMfVxJoG3wsWFO7t1w@mail.gmail.com>
Hi Ard,
On Tue, Sep 18, 2018 at 11:28:50AM -0700, Ard Biesheuvel wrote:
> On 18 September 2018 at 09:16, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > - While I initially wasn't going to do this for the initial
> > patchset, it was just so simple to do: now there's a nosimd
> > module parameter that can be used to disable simd instructions
> > for debugging and testing, or on weird systems.
> >
>
> I was going to respond in the other thread but it is probably better
> to move the discussion here.
>
> My concern about the monolithic nature of each algo module is not only
> about SIMD, and it has nothing to do with weird systems. It has to do
> with micro-architectural differences which are more common on ARM than
> on other architectures *, I suppose. But generalizing from that, it
> has to do with policy which is currently owned by userland and not by
> the kernel. This will also be important for choosing between the time
> variant but less safe table based scalar AES and the much slower time
> invariant version (which is substantially slower, especially on
> decryption) once we move AES into this library.
>
> So a command line option for the kernel is not the solution here. If
> we can't have separate modules, could we at least have per-module
> options that put the policy decisions back into userland?
>
> * as an example, the SHA256 NEON code I collaborated on with Andy
> Polyakov 2 years ago is significantly faster on some cores and not on
> others
Interesting concern. There are micro-architectural quirks on x86 too
that the current code actually already considers. Notably, we use an
AVX-512VL path for Skylake-X but an AVX-512F path for Knights Landing
and Coffee Lake and others, due to thermal throttling when touching the
zmm registers on Skylake-X. So, in the code, we have it automatically
select the right thing based on the micro-architecture.
Is the same thing not possible with ARM? Do you not have access to this
information already, such that the module can just always do the right
thing and not require any user intervention?
If so, that would be ideal. If not (and I'm curious to learn why not
exactly), then indeed we could add some runtime nobs in /sys/module/
{algo}/parameters/{nob}, or the like. This would be super easy to do,
should we ever encounter a situation where we're unable to auto-detect
the correct thing.
Regards,
Jason
^ 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