* [net-next 01/14] i40evf: Use net_device_stats from struct net_device
From: Jeff Kirsher @ 2017-04-20 1:57 UTC (permalink / raw)
To: davem; +Cc: Tobias Klauser, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170420015750.6828-1-jeffrey.t.kirsher@intel.com>
From: Tobias Klauser <tklauser@distanz.ch>
Instead of using a private copy of struct net_device_stats in
struct i40evf_adapter, use stats from struct net_device. Also remove the
now unnecessary .ndo_get_stats function.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40evf/i40evf.h | 1 -
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 16 ----------------
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 22 +++++++++++-----------
3 files changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index 35ded19e9cc2..7bbd11abed08 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -252,7 +252,6 @@ struct i40evf_adapter {
/* OS defined structs */
struct net_device *netdev;
struct pci_dev *pdev;
- struct net_device_stats net_stats;
struct i40e_hw hw; /* defined in i40e_type.h */
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 12a930e879af..671913c74bf8 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -2243,21 +2243,6 @@ static int i40evf_close(struct net_device *netdev)
}
/**
- * i40evf_get_stats - Get System Network Statistics
- * @netdev: network interface device structure
- *
- * Returns the address of the device statistics structure.
- * The statistics are actually updated from the timer callback.
- **/
-static struct net_device_stats *i40evf_get_stats(struct net_device *netdev)
-{
- struct i40evf_adapter *adapter = netdev_priv(netdev);
-
- /* only return the current stats */
- return &adapter->net_stats;
-}
-
-/**
* i40evf_change_mtu - Change the Maximum Transfer Unit
* @netdev: network interface device structure
* @new_mtu: new value for maximum frame size
@@ -2363,7 +2348,6 @@ static const struct net_device_ops i40evf_netdev_ops = {
.ndo_open = i40evf_open,
.ndo_stop = i40evf_close,
.ndo_start_xmit = i40evf_xmit_frame,
- .ndo_get_stats = i40evf_get_stats,
.ndo_set_rx_mode = i40evf_set_rx_mode,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = i40evf_set_mac,
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 3bccfbb1db14..deb2cb8dac6b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -960,17 +960,17 @@ void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
case I40E_VIRTCHNL_OP_GET_STATS: {
struct i40e_eth_stats *stats =
(struct i40e_eth_stats *)msg;
- adapter->net_stats.rx_packets = stats->rx_unicast +
- stats->rx_multicast +
- stats->rx_broadcast;
- adapter->net_stats.tx_packets = stats->tx_unicast +
- stats->tx_multicast +
- stats->tx_broadcast;
- adapter->net_stats.rx_bytes = stats->rx_bytes;
- adapter->net_stats.tx_bytes = stats->tx_bytes;
- adapter->net_stats.tx_errors = stats->tx_errors;
- adapter->net_stats.rx_dropped = stats->rx_discards;
- adapter->net_stats.tx_dropped = stats->tx_discards;
+ netdev->stats.rx_packets = stats->rx_unicast +
+ stats->rx_multicast +
+ stats->rx_broadcast;
+ netdev->stats.tx_packets = stats->tx_unicast +
+ stats->tx_multicast +
+ stats->tx_broadcast;
+ netdev->stats.rx_bytes = stats->rx_bytes;
+ netdev->stats.tx_bytes = stats->tx_bytes;
+ netdev->stats.tx_errors = stats->tx_errors;
+ netdev->stats.rx_dropped = stats->rx_discards;
+ netdev->stats.tx_dropped = stats->tx_discards;
adapter->current_stats = *stats;
}
break;
--
2.12.2
^ permalink raw reply related
* [net-next 00/14][pull request] 40GbE Intel Wired LAN Driver Updates 2017-04-19
From: Jeff Kirsher @ 2017-04-20 1:57 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to i40e and i40evf only, most notable being
the addition of trace points for BPF programs.
Tobias Klauser updates i40evf to use net_device stats struct instead
of a local private copy.
Preethi updates the VF driver to not enable receive checksum offload by
default for tunneled packets.
Alex fixes an issue he introduced when he converted the code over to
using the length field to determine if a descriptor was done or not.
Mitch adds the ability to dump additional information on the VFs, which
is not available through 'ip link show' using debugfs.
Scott adds trace points to the drivers so that BPF programs can be
attached for feature testing and verification.
Jingjing adds admin queue functions for Pipeline Personalization Profile
commands.
Jake does most of the heavy lifting in this series, starting with the
a reduction in the scope of the RTNL lock being held while resetting VFs
to allow multiple PFs to reset in a timely manner. Factored out the
direct queue modification so that we are able to re-use the code.
Reduced the wait time for admin queue commands to complete, since we were
waiting a minimum of a millisecond, when in practice the admin queue
command is processed often much faster. Cleaned up code (flag) we never
use. Make the code to resetting all the VFs optimized for parallel
computing instead of the current way is a serialized fashion, to help
reduce the time it takes.
The following are changes since commit 9868879f293c599ce13b584c5bd8800312970781:
net: cx89x0: move attribute declaration before struct keyword
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE
Alexander Duyck (1):
i40e: Fix support for flow director programming status
Jacob Keller (8):
i40e: don't hold RTNL lock while waiting for VF reset to finish
i40e: factor out queue control from i40e_vsi_control_(tx|rx)
i40e: fix CONFIG_BUSY checks in i40e_set_settings function
i40e: reduce wait time for adminq command completion
i40e: remove I40E_FLAG_IN_NETPOLL entirely
i40e: split some code in i40e_reset_vf into helpers
i40e: reset all VFs in parallel when rebuilding PF
i40e: use i40e_stop_rings_no_wait to implement PORT_SUSPENDED state
Jingjing Wu (1):
i40e: new AQ commands
Mitch Williams (1):
i40e: dump VF information in debugfs
Scott Peterson (1):
i40e/i40evf: Add tracepoints
Tobias Klauser (1):
i40evf: Use net_device_stats from struct net_device
alice michael (1):
i40e/i40evf: Remove VF Rx csum offload for tunneled packets
drivers/net/ethernet/intel/i40e/Makefile | 3 +
drivers/net/ethernet/intel/i40e/i40e.h | 19 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 4 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h | 34 +++
drivers/net/ethernet/intel/i40e/i40e_common.c | 212 +++++++++++++++++++
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 51 +++++
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 38 +++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 200 ++++++++++++------
drivers/net/ethernet/intel/i40e/i40e_prototype.h | 17 ++
drivers/net/ethernet/intel/i40e/i40e_trace.h | 229 +++++++++++++++++++++
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 59 ++++--
drivers/net/ethernet/intel/i40e/i40e_type.h | 80 +++++++
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 209 +++++++++++++++----
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 1 +
drivers/net/ethernet/intel/i40evf/Makefile | 3 +
drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 4 +-
drivers/net/ethernet/intel/i40evf/i40e_adminq.h | 2 +-
.../net/ethernet/intel/i40evf/i40e_adminq_cmd.h | 34 +++
drivers/net/ethernet/intel/i40evf/i40e_common.c | 212 +++++++++++++++++++
drivers/net/ethernet/intel/i40evf/i40e_prototype.h | 17 ++
drivers/net/ethernet/intel/i40evf/i40e_trace.h | 229 +++++++++++++++++++++
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 25 ++-
drivers/net/ethernet/intel/i40evf/i40e_type.h | 80 +++++++
drivers/net/ethernet/intel/i40evf/i40evf.h | 3 -
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 23 +--
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 22 +-
27 files changed, 1625 insertions(+), 187 deletions(-)
create mode 100644 drivers/net/ethernet/intel/i40e/i40e_trace.h
create mode 100644 drivers/net/ethernet/intel/i40evf/i40e_trace.h
--
2.12.2
^ permalink raw reply
* Re: [PATCH 1/2] openvswitch: Typo fix.
From: Jarno Rajahalme @ 2017-04-20 1:56 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: netdev
In-Reply-To: <1492652976-99201-1-git-send-email-jarno@ovn.org>
Sorry for the chatter, forgot to include “net-next” in the title, sending again.
Jarno
> On Apr 19, 2017, at 6:49 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>
> Fix typo in a comment.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> net/openvswitch/conntrack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 7b2c2fc..58de4c2 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -373,7 +373,7 @@ static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
> }
>
> /* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
> - * IPCT_LABEL bit it set in the event cache.
> + * IPCT_LABEL bit is set in the event cache.
> */
> nf_conntrack_event_cache(IPCT_LABEL, ct);
>
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH 2/2] openvswitch: Add eventmask support to CT action.
From: Jarno Rajahalme @ 2017-04-20 1:49 UTC (permalink / raw)
To: netdev; +Cc: jarno
In-Reply-To: <1492652976-99201-1-git-send-email-jarno@ovn.org>
Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
which can be used in conjunction with the commit flag
(OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
conntrack events (IPCT_*) should be delivered via the Netfilter
netlink multicast groups. Default behavior depends on the system
configuration, but typically a lot of events are delivered. This can be
very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
types of events are of interest.
Netfilter core init_conntrack() adds the event cache extension, so we
only need to set the ctmask value. However, if the system is
configured without support for events, the setting will be skipped due
to extension not being found.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
include/uapi/linux/openvswitch.h | 12 ++++++++++++
net/openvswitch/conntrack.c | 27 +++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 66d1c3c..38ae95d 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -693,6 +693,17 @@ struct ovs_action_hash {
* nothing if the connection is already committed will check that the current
* packet is in conntrack entry's original direction. If directionality does
* not match, will delete the existing conntrack entry and commit a new one.
+ * @OVS_CT_ATTR_EVENTMASK: Mask of bits indicating which conntrack event types
+ * (enum ip_conntrack_events IPCT_*) should be reported. For any bit set to
+ * zero, the corresponding event type is not generated. Default behavior
+ * depends on system configuration, but typically all event types are
+ * generated, hence listening on UPDATE events may get a lot of events.
+ * Explicitly passing this attribute allows limiting the updates received to
+ * the events of interest. The bit 1 << IPCT_NEW, 1 << IPCT_RELATED, and
+ * 1 << IPCT_DESTROY must be set to ones for those events to be received on
+ * NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups, respectively.
+ * Remaining bits control the changes for which an event is delivered on the
+ * NFNLGRP_CONNTRACK_UPDATE group.
*/
enum ovs_ct_attr {
OVS_CT_ATTR_UNSPEC,
@@ -704,6 +715,7 @@ enum ovs_ct_attr {
related connections. */
OVS_CT_ATTR_NAT, /* Nested OVS_NAT_ATTR_* */
OVS_CT_ATTR_FORCE_COMMIT, /* No argument */
+ OVS_CT_ATTR_EVENTMASK, /* u32 mask of IPCT_* events. */
__OVS_CT_ATTR_MAX
};
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 58de4c2..4f7c3b5 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -66,7 +66,9 @@ struct ovs_conntrack_info {
u8 commit : 1;
u8 nat : 3; /* enum ovs_ct_nat */
u8 force : 1;
+ u8 have_eventmask : 1;
u16 family;
+ u32 eventmask; /* Mask of 1 << IPCT_*. */
struct md_mark mark;
struct md_labels labels;
#ifdef CONFIG_NF_NAT_NEEDED
@@ -1007,6 +1009,20 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
if (!ct)
return 0;
+ /* Set the conntrack event mask if given. NEW and DELETE events have
+ * their own groups, but the NFNLGRP_CONNTRACK_UPDATE group listener
+ * typically would receive many kinds of updates. Setting the event
+ * mask allows those events to be filtered. The set event mask will
+ * remain in effect for the lifetime of the connection unless changed
+ * by a further CT action with both the commit flag and the eventmask
+ * option. */
+ if (info->have_eventmask) {
+ struct nf_conntrack_ecache *cache = nf_ct_ecache_find(ct);
+
+ if (cache)
+ cache->ctmask = info->eventmask;
+ }
+
/* Apply changes before confirming the connection so that the initial
* conntrack NEW netlink event carries the values given in the CT
* action.
@@ -1238,6 +1254,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
/* NAT length is checked when parsing the nested attributes. */
[OVS_CT_ATTR_NAT] = { .minlen = 0, .maxlen = INT_MAX },
#endif
+ [OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32),
+ .maxlen = sizeof(u32) },
};
static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
@@ -1316,6 +1334,11 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
break;
}
#endif
+ case OVS_CT_ATTR_EVENTMASK:
+ info->have_eventmask = true;
+ info->eventmask = nla_get_u32(a);
+ break;
+
default:
OVS_NLERR(log, "Unknown conntrack attr (%d)",
type);
@@ -1515,6 +1538,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
ct_info->helper->name))
return -EMSGSIZE;
}
+ if (ct_info->have_eventmask &&
+ nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
+ return -EMSGSIZE;
+
#ifdef CONFIG_NF_NAT_NEEDED
if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb))
return -EMSGSIZE;
--
2.1.4
^ permalink raw reply related
* [PATCH 1/2] openvswitch: Typo fix.
From: Jarno Rajahalme @ 2017-04-20 1:49 UTC (permalink / raw)
To: netdev; +Cc: jarno
Fix typo in a comment.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
net/openvswitch/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 7b2c2fc..58de4c2 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -373,7 +373,7 @@ static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
}
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
- * IPCT_LABEL bit it set in the event cache.
+ * IPCT_LABEL bit is set in the event cache.
*/
nf_conntrack_event_cache(IPCT_LABEL, ct);
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/2] openvswitch: Add eventmask support to CT action.
From: Jarno Rajahalme @ 2017-04-20 1:55 UTC (permalink / raw)
To: netdev; +Cc: jarno
In-Reply-To: <1492653311-99269-1-git-send-email-jarno@ovn.org>
Add a new optional conntrack action attribute OVS_CT_ATTR_EVENTMASK,
which can be used in conjunction with the commit flag
(OVS_CT_ATTR_COMMIT) to set the mask of bits specifying which
conntrack events (IPCT_*) should be delivered via the Netfilter
netlink multicast groups. Default behavior depends on the system
configuration, but typically a lot of events are delivered. This can be
very chatty for the NFNLGRP_CONNTRACK_UPDATE group, even if only some
types of events are of interest.
Netfilter core init_conntrack() adds the event cache extension, so we
only need to set the ctmask value. However, if the system is
configured without support for events, the setting will be skipped due
to extension not being found.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
include/uapi/linux/openvswitch.h | 12 ++++++++++++
net/openvswitch/conntrack.c | 27 +++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 66d1c3c..38ae95d 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -693,6 +693,17 @@ struct ovs_action_hash {
* nothing if the connection is already committed will check that the current
* packet is in conntrack entry's original direction. If directionality does
* not match, will delete the existing conntrack entry and commit a new one.
+ * @OVS_CT_ATTR_EVENTMASK: Mask of bits indicating which conntrack event types
+ * (enum ip_conntrack_events IPCT_*) should be reported. For any bit set to
+ * zero, the corresponding event type is not generated. Default behavior
+ * depends on system configuration, but typically all event types are
+ * generated, hence listening on UPDATE events may get a lot of events.
+ * Explicitly passing this attribute allows limiting the updates received to
+ * the events of interest. The bit 1 << IPCT_NEW, 1 << IPCT_RELATED, and
+ * 1 << IPCT_DESTROY must be set to ones for those events to be received on
+ * NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups, respectively.
+ * Remaining bits control the changes for which an event is delivered on the
+ * NFNLGRP_CONNTRACK_UPDATE group.
*/
enum ovs_ct_attr {
OVS_CT_ATTR_UNSPEC,
@@ -704,6 +715,7 @@ enum ovs_ct_attr {
related connections. */
OVS_CT_ATTR_NAT, /* Nested OVS_NAT_ATTR_* */
OVS_CT_ATTR_FORCE_COMMIT, /* No argument */
+ OVS_CT_ATTR_EVENTMASK, /* u32 mask of IPCT_* events. */
__OVS_CT_ATTR_MAX
};
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 58de4c2..4f7c3b5 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -66,7 +66,9 @@ struct ovs_conntrack_info {
u8 commit : 1;
u8 nat : 3; /* enum ovs_ct_nat */
u8 force : 1;
+ u8 have_eventmask : 1;
u16 family;
+ u32 eventmask; /* Mask of 1 << IPCT_*. */
struct md_mark mark;
struct md_labels labels;
#ifdef CONFIG_NF_NAT_NEEDED
@@ -1007,6 +1009,20 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
if (!ct)
return 0;
+ /* Set the conntrack event mask if given. NEW and DELETE events have
+ * their own groups, but the NFNLGRP_CONNTRACK_UPDATE group listener
+ * typically would receive many kinds of updates. Setting the event
+ * mask allows those events to be filtered. The set event mask will
+ * remain in effect for the lifetime of the connection unless changed
+ * by a further CT action with both the commit flag and the eventmask
+ * option. */
+ if (info->have_eventmask) {
+ struct nf_conntrack_ecache *cache = nf_ct_ecache_find(ct);
+
+ if (cache)
+ cache->ctmask = info->eventmask;
+ }
+
/* Apply changes before confirming the connection so that the initial
* conntrack NEW netlink event carries the values given in the CT
* action.
@@ -1238,6 +1254,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
/* NAT length is checked when parsing the nested attributes. */
[OVS_CT_ATTR_NAT] = { .minlen = 0, .maxlen = INT_MAX },
#endif
+ [OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32),
+ .maxlen = sizeof(u32) },
};
static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
@@ -1316,6 +1334,11 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
break;
}
#endif
+ case OVS_CT_ATTR_EVENTMASK:
+ info->have_eventmask = true;
+ info->eventmask = nla_get_u32(a);
+ break;
+
default:
OVS_NLERR(log, "Unknown conntrack attr (%d)",
type);
@@ -1515,6 +1538,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
ct_info->helper->name))
return -EMSGSIZE;
}
+ if (ct_info->have_eventmask &&
+ nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
+ return -EMSGSIZE;
+
#ifdef CONFIG_NF_NAT_NEEDED
if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb))
return -EMSGSIZE;
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 1/2] openvswitch: Typo fix.
From: Jarno Rajahalme @ 2017-04-20 1:55 UTC (permalink / raw)
To: netdev; +Cc: jarno
Fix typo in a comment.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
net/openvswitch/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 7b2c2fc..58de4c2 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -373,7 +373,7 @@ static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
}
/* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
- * IPCT_LABEL bit it set in the event cache.
+ * IPCT_LABEL bit is set in the event cache.
*/
nf_conntrack_event_cache(IPCT_LABEL, ct);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next v6 05/11] seccomp: Split put_seccomp_filter() with put_seccomp()
From: Kees Cook @ 2017-04-20 1:54 UTC (permalink / raw)
To: Mickaël Salaün
Cc: LKML, Alexei Starovoitov, Andy Lutomirski,
Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
David Drysdale, David S . Miller, Eric W . Biederman,
James Morris, Jann Horn, Jonathan Corbet, Matthew Garrett,
Michael Kerrisk, Paul Moore, Sargun Dhillon, Serge E . Hallyn,
Shuah Khan, Tejun Heo, Thomas Graf, Will Drewry <wa
In-Reply-To: <96024881-1bcc-33af-6285-d9a904de963e-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
On Wed, Apr 19, 2017 at 3:18 PM, Mickaël Salaün <mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org> wrote:
>
> On 19/04/2017 00:47, Mickaël Salaün wrote:
>>
>> On 19/04/2017 00:23, Kees Cook wrote:
>>> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>> The semantic is unchanged. This will be useful for the Landlock
>>>> integration with seccomp (next commit).
>>>>
>>>> Signed-off-by: Mickaël Salaün <mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
>>>> Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>>> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>>>> Cc: Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>>> ---
>>>> include/linux/seccomp.h | 4 ++--
>>>> kernel/fork.c | 2 +-
>>>> kernel/seccomp.c | 18 +++++++++++++-----
>>>> 3 files changed, 16 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
>>>> index ecc296c137cd..e25aee2cdfc0 100644
>>>> --- a/include/linux/seccomp.h
>>>> +++ b/include/linux/seccomp.h
>>>> @@ -77,10 +77,10 @@ static inline int seccomp_mode(struct seccomp *s)
>>>> #endif /* CONFIG_SECCOMP */
>>>>
>>>> #ifdef CONFIG_SECCOMP_FILTER
>>>> -extern void put_seccomp_filter(struct task_struct *tsk);
>>>> +extern void put_seccomp(struct task_struct *tsk);
>>>> extern void get_seccomp_filter(struct task_struct *tsk);
>>>> #else /* CONFIG_SECCOMP_FILTER */
>>>> -static inline void put_seccomp_filter(struct task_struct *tsk)
>>>> +static inline void put_seccomp(struct task_struct *tsk)
>>>> {
>>>> return;
>>>> }
>>>> diff --git a/kernel/fork.c b/kernel/fork.c
>>>> index 6c463c80e93d..a27d8e67ce33 100644
>>>> --- a/kernel/fork.c
>>>> +++ b/kernel/fork.c
>>>> @@ -363,7 +363,7 @@ void free_task(struct task_struct *tsk)
>>>> #endif
>>>> rt_mutex_debug_task_free(tsk);
>>>> ftrace_graph_exit_task(tsk);
>>>> - put_seccomp_filter(tsk);
>>>> + put_seccomp(tsk);
>>>> arch_release_task_struct(tsk);
>>>> if (tsk->flags & PF_KTHREAD)
>>>> free_kthread_struct(tsk);
>>>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>>>> index 65f61077ad50..326f79e32127 100644
>>>> --- a/kernel/seccomp.c
>>>> +++ b/kernel/seccomp.c
>>>> @@ -64,6 +64,8 @@ struct seccomp_filter {
>>>> /* Limit any path through the tree to 256KB worth of instructions. */
>>>> #define MAX_INSNS_PER_PATH ((1 << 18) / sizeof(struct sock_filter))
>>>>
>>>> +static void put_seccomp_filter(struct seccomp_filter *filter);
>>>
>>> Can this be reorganized easily to avoid a forward-declaration?
>>
>> I didn't want to move too much code but I will.
>>
>>>
>>>> +
>>>> /*
>>>> * Endianness is explicitly ignored and left for BPF program authors to manage
>>>> * as per the specific architecture.
>>>> @@ -314,7 +316,7 @@ static inline void seccomp_sync_threads(void)
>>>> * current's path will hold a reference. (This also
>>>> * allows a put before the assignment.)
>>>> */
>>>> - put_seccomp_filter(thread);
>>>> + put_seccomp_filter(thread->seccomp.filter);
>>>> smp_store_release(&thread->seccomp.filter,
>>>> caller->seccomp.filter);
>>>>
>>>> @@ -476,10 +478,11 @@ static inline void seccomp_filter_free(struct seccomp_filter *filter)
>>>> }
>>>> }
>>>>
>>>> -/* put_seccomp_filter - decrements the ref count of tsk->seccomp.filter */
>>>> -void put_seccomp_filter(struct task_struct *tsk)
>>>> +/* put_seccomp_filter - decrements the ref count of a filter */
>>>> +static void put_seccomp_filter(struct seccomp_filter *filter)
>>>> {
>>>> - struct seccomp_filter *orig = tsk->seccomp.filter;
>>>> + struct seccomp_filter *orig = filter;
>>>> +
>>>> /* Clean up single-reference branches iteratively. */
>>>> while (orig && atomic_dec_and_test(&orig->usage)) {
>>>> struct seccomp_filter *freeme = orig;
>>>> @@ -488,6 +491,11 @@ void put_seccomp_filter(struct task_struct *tsk)
>>>> }
>>>> }
>>>>
>>>> +void put_seccomp(struct task_struct *tsk)
>>>> +{
>>>> + put_seccomp_filter(tsk->seccomp.filter);
>>>> +}
>>>> +
>>>> static void seccomp_init_siginfo(siginfo_t *info, int syscall, int reason)
>>>> {
>>>> memset(info, 0, sizeof(*info));
>>>> @@ -914,7 +922,7 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
>>>> if (copy_to_user(data, fprog->filter, bpf_classic_proglen(fprog)))
>>>> ret = -EFAULT;
>>>>
>>>> - put_seccomp_filter(task);
>>>> + put_seccomp_filter(task->seccomp.filter);
>>>> return ret;
>>>
>>> I don't like that the arguments to get_seccomp_filter() and
>>> put_seccomp_filter() are now different. I think they should match for
>>> readability.
>>
>> OK, I can do that.
>>
>
> Kees, can I send this as a separate patch?
Sure! Though I still think the argument to get/put_seccomp_filter()
should be task_struct.
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH net-next v6 09/11] seccomp: Enhance test_harness with an assert step mechanism
From: Kees Cook @ 2017-04-20 1:50 UTC (permalink / raw)
To: Mickaël Salaün, Shuah Khan
Cc: LKML, Alexei Starovoitov, Andy Lutomirski,
Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
David Drysdale, David S . Miller, Eric W . Biederman,
James Morris, Jann Horn, Jonathan Corbet, Matthew Garrett,
Michael Kerrisk, Paul Moore, Sargun Dhillon, Serge E . Hallyn,
Tejun Heo, Thomas Graf, Will Drewry, "kernel-harden
In-Reply-To: <df0e6143-6db3-b7aa-f239-e879455316fa@digikod.net>
On Wed, Apr 19, 2017 at 3:05 PM, Mickaël Salaün <mic@digikod.net> wrote:
>
>
> On 20/04/2017 00:02, Kees Cook wrote:
>> On Wed, Apr 19, 2017 at 2:51 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>
>>> On 19/04/2017 02:02, Kees Cook wrote:
>>>> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>>> This is useful to return an information about the error without being
>>>>> able to write to TH_LOG_STREAM.
>>>>>
>>>>> Helpers from test_harness.h may be useful outside of the seccomp
>>>>> directory.
>>>>>
>>>>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>>>>> Cc: Andy Lutomirski <luto@amacapital.net>
>>>>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>>>>> Cc: Kees Cook <keescook@chromium.org>
>>>>> Cc: Shuah Khan <shuah@kernel.org>
>>>>> Cc: Will Drewry <wad@chromium.org>
>>>>> ---
>>>>> tools/testing/selftests/seccomp/test_harness.h | 8 +++++++-
>>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/seccomp/test_harness.h b/tools/testing/selftests/seccomp/test_harness.h
>>>>> index a786c69c7584..77e407663e06 100644
>>>>> --- a/tools/testing/selftests/seccomp/test_harness.h
>>>>> +++ b/tools/testing/selftests/seccomp/test_harness.h
>>>>> @@ -397,7 +397,7 @@ struct __test_metadata {
>>>>> const char *name;
>>>>> void (*fn)(struct __test_metadata *);
>>>>> int termsig;
>>>>> - int passed;
>>>>> + __s8 passed;
>>>>
>>>> Why the reduction here? int is signed too?
>>>
>>> Because the return code of a process is capped to 8 bits and I use a
>>> negative value to not mess with the current interpretation of 0 (error)
>>> and 1 (OK) for the "passed" variable.
>>>
>>>>
>>>>> int trigger; /* extra handler after the evaluation */
>>>>> struct __test_metadata *prev, *next;
>>>>> };
>>>>> @@ -476,6 +476,12 @@ void __run_test(struct __test_metadata *t)
>>>>> "instead of by signal (code: %d)\n",
>>>>> t->name,
>>>>> WEXITSTATUS(status));
>>>>> + } else if (t->passed < 0) {
>>>>> + fprintf(TH_LOG_STREAM,
>>>>> + "%s: Failed at step #%d\n",
>>>>> + t->name,
>>>>> + t->passed * -1);
>>>>> + t->passed = 0;
>>>>> }
>>>>
>>>> Instead of creating an overloaded mechanism here, perhaps have an
>>>> option reporting mechanism that can be enabled. Like adding to
>>>> __test_metadata "bool no_stream; int test_number;" and adding
>>>> test_number++ to each ASSERT/EXCEPT call, and doing something like:
>>>>
>>>> if (t->no_stream) {
>>>> fprintf(TH_LOG_STREAM,
>>>> "%s: Failed at step #%d\n",
>>>> t->name,
>>>> t->test_number);
>>>> }
>>>>
>>>> It'd be a cleaner approach, maybe?
>>>
>>> Good idea, we will then be able to use 255 steps!
>>>
>>> Do you want me to send this as a separate patch?
>>>
>>> Can we move test_harness.h outside of the seccomp directory to be
>>> available to other subsystems as well?
>>
>> Yeah, I would do two patches, and send them out separately (to shuah
>> with lkml and me in cc at least), one to move test_hardness.h into
>> some include/ directory, and then to add the new logic for streamless
>> reporting.
>>
>> Thanks!
>>
>
> Good, in which place and name would it fit better?
I've added Shuah to CC. Shuah, where should a common header file for
selftests live? Should a new "include" directory be added?
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem
From: Kees Cook @ 2017-04-20 1:48 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Casey Schaufler, LKML, Alexei Starovoitov, Andy Lutomirski,
Arnaldo Carvalho de Melo, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
Jonathan Corbet, Matthew Garrett, Michael Kerrisk, Paul Moore,
Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Tejun Heo,
Thomas Graf, Will Drewry <wa
In-Reply-To: <35272f2b-ec5f-d032-ae2e-9fc0b4c0e2e3@digikod.net>
On Wed, Apr 19, 2017 at 3:03 PM, Mickaël Salaün <mic@digikod.net> wrote:
>
> On 19/04/2017 01:40, Kees Cook wrote:
>> On Tue, Apr 18, 2017 at 4:16 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 4/18/2017 3:44 PM, Mickaël Salaün wrote:
>>>> On 19/04/2017 00:17, Kees Cook wrote:
>>>>> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic@digikod.net> wrote:
>>>>>> +void __init landlock_add_hooks(void)
>>>>>> +{
>>>>>> + pr_info("landlock: Version %u", LANDLOCK_VERSION);
>>>>>> + landlock_add_hooks_fs();
>>>>>> + security_add_hooks(NULL, 0, "landlock");
>>>>>> + bpf_register_prog_type(&bpf_landlock_type);
>>>>> I'm confused by the separation of hook registration here. The call to
>>>>> security_add_hooks is with count=0 is especially weird. Why isn't this
>>>>> just a single call with security_add_hooks(landlock_hooks,
>>>>> ARRAY_SIZE(landlock_hooks), "landlock")?
>>>> Yes, this is ugly with the new security_add_hooks() with three arguments
>>>> but I wanted to split the hooks definition in multiple files.
>>>
>>> Why? I'll buy a good argument, but there are dangers in
>>> allowing multiple calls to security_add_hooks().
>
> I prefer to have one file per hook "family" (e.g. filesystem, network,
> ptrace…). This reduce the mess with all the included files (needed for
> LSM hook argument types) and make the files easier to read, understand
> and maintain.
>
>>>
>>>>
>>>> The current security_add_hooks() use lsm_append(lsm, &lsm_names) which
>>>> is not exported. Unfortunately, calling multiple security_add_hooks()
>>>> with the same LSM name would register multiple names for the same LSM…
>>>> Is it OK if I modify this function to not add duplicated entries?
>>>
>>> It may seem absurd, but it's conceivable that a module might
>>> have two hooks it wants called. My example is a module that
>>> counts the number of times SELinux denies a process access to
>>> things (which needs to be called before and after SELinux in
>>> order to detect denials) and takes "appropriate action" if
>>> too many denials occur. It would be weird, wonky and hackish,
>>> but that never stopped anybody before.
>
> Right, but now, with the new lsm_append(), module names are concatenated
> ("%s,%s") in the lsm_names variable. It would be nice to not pollute
> this string with multiple time the same module name.
Perhaps security_add_hooks could be modified to accept a NULL lsm to
skip the lsm_append() call, so it could do:
security_add_hooks(hooks1, count1, NULL);
security_add_hooks(hooks2, count2, NULL);
security_add_hooks(NULL, 0, "landlock");
Or, as Casey suggests, disregard adding the name when it already exists:
security_add_hooks(hooks1, count1, "landlock");
security_add_hooks(hooks2, count2, "landlock");
Yeah, I think I prefer this...
-Kees
>
>>
>> If ends up being sane and clear, I'm fine with allowing multiple calls.
>>
>> -Kees
>>
>
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH v4 net-next RFC] net: Generic XDP
From: David Miller @ 2017-04-20 1:40 UTC (permalink / raw)
To: andy; +Cc: alexei.starovoitov, michael.chan, netdev, xdp-newbies
In-Reply-To: <20170419142903.GJ4730@C02RW35GFVH8.dhcp.broadcom.net>
From: Andy Gospodarek <andy@greyhouse.net>
Date: Wed, 19 Apr 2017 10:29:03 -0400
> So I tried a variety of things and the simplest change on top of yours that
> works well for xdp1, xdp2, and xdp_tx_iptunnel.
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b3d3a6e..1bab3dc 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4316,11 +4316,11 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
>
> off = xdp.data - orig_data;
> if (off)
> - __skb_push(skb, off);
> + __skb_push(skb, -off);
We have to handle both pushing and popping headers, so could you
please test the snippet I asked you to try?
> if (off > 0)
> __skb_pull(skb, off);
> else if (off < 0)
> __skb_push(skb, -off);
Thanks.
^ permalink raw reply
* RE: IT Newsletter
From: Gary Knight @ 2017-04-20 1:01 UTC (permalink / raw)
To: Gary Knight
In-Reply-To: <E3A03F19D611AA4F99051CC9E30A8994025B6D90E2@FBA01MBX02.fa.ds.com>
________________________________
From: Gary Knight
Sent: 20 April 2017 01:10
Subject: IT Newsletter
Dear colleagues,
To keep you abreast of ICT developments of the Organization and to keep your technical skills up to date, the latest IT Newsletter issue is now available at <http://9yu5dzi27.ugo.florist/>
http://www.tjyukilohgchgcj.citymax.com/feedback.html
ICT Service Desk | World Intellectual Property Organization
P Please consider the environment before printing this e-mail
This communication contains information which is confidential, which may be privileged, and which is for the exclusive use of the intended recipient(s). If you are not an intended recipient please note that any distribution, disclosure, use or copying of any
part of this communication is strictly prohibited. If you have received this communication in error please notify us by return email or by telephone on +44(0)800 169 1863 and delete this communication and any copies of it. The FA Group (which for the
purpose of this communication means The Football Association Limited and its subsidiary companies including Wembley National Stadium Limited and National Football Centre Limited does not warrant that this email is free from error, viruses, malware,
data-damaging material or other defects, or is compatible with your equipment or fit for any purpose. The FA Group may monitor, intercept and block emails addressed to its users or take any other action in accordance with its email use policy.
Statements or opinions may be expressed in this communication that are personal to the sender and do not necessarily represent the views of The FA Group or any member of it. Unless expressly stated otherwise, no member of The FA Group shall be
bound by any contract or obligation purported to be created by this communication.
This communication has originated from the communications system of The FA Group.
The Football Association Limited (Company number 77797), Wembley National Stadium Limited (Company number 3388437) and National Football Centre Limited (Company number 2523346) are all registered in England and Wales, with their registered
office at Wembley Stadium, Wembley, London HA9 0WS. For The FA Tel: +44(0)800 169 1863. http://www.thefa.com. For Wembley National Stadium Limited Tel: +44(0)800 169 2007 http://www.wembleystadium.com
^ permalink raw reply
* [PATCH net] netdevice: Prefer NETIF_F_HW_CSUM when intersecting features
From: Vladislav Yasevich @ 2017-04-20 1:12 UTC (permalink / raw)
To: netdev; +Cc: tom, mkubecek, Vladislav Yasevich
While hardware device use either NETIF_F_(IP|IPV6)_CSUM or
NETIF_F_HW_CSUM, all of the software devices use HW_CSUM.
This results in an interesting situation when the software
device is configured on top of hw device using (IP|IPV6)_CSUM.
In this situation, the user can't turn off checksum offloading
features on the software device.
This patch resolves that by prefering the NETIF_F_HW_CSUM setting
when computing a feature intersect.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 97456b25..3d811c1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4019,9 +4019,9 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
{
if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
if (f1 & NETIF_F_HW_CSUM)
- f1 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
+ f2 |= NETIF_F_HW_CSUM;
else
- f2 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
+ f1 |= NETIF_F_HW_CSUM;
}
return f1 & f2;
--
2.7.4
^ permalink raw reply related
* Re: __sk_buff.data_end
From: Daniel Borkmann @ 2017-04-20 0:38 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Johannes Berg, Alexei Starovoitov, netdev
In-Reply-To: <20170420001218.GA38173@ast-mbp.thefacebook.com>
On 04/20/2017 02:12 AM, Alexei Starovoitov wrote:
> On Thu, Apr 20, 2017 at 02:01:49AM +0200, Daniel Borkmann wrote:
>> On 04/20/2017 12:20 AM, Johannes Berg wrote:
>>> On Wed, 2017-04-19 at 23:31 +0200, Johannes Berg wrote:
>>>> Hi Alexei, Daniel,
>>>>
>>>> I'm looking at adding the __wifi_sk_buff I talked about, and I notice
>>>> that it uses CB space to store data_end. Unfortunately, in a lot of
>>>> cases, we don't have any CB space to spare in wifi.
>>>
>>> I guess I can work around this, would this seem reasonable?
>>>
>>> struct bpf_skb_data_end {
>>> struct qdisc_skb_cb qdisc_cb;
>>> - void *data_end;
>>> + /*
>>> + * The alignment here is for mac80211, since that doesn't use
>>> + * a pointer but a u64 value and needs to save/restore that
>>> + * across running its BPF programs.
>>> + */
>>> + void *data_end __aligned(sizeof(u64));
>>> };
>>
>> Yeah, should work as well for the 32 bit archs, on 64 bit we
>> have this effectively already:
>>
>> struct bpf_skb_data_end {
>> struct qdisc_skb_cb qdisc_cb; /* 0 28 */
>>
>> /* XXX 4 bytes hole, try to pack */
>>
>> void * data_end; /* 32 8 */
>>
>> /* size: 40, cachelines: 1, members: 2 */
>> /* sum members: 36, holes: 1, sum holes: 4 */
>> /* last cacheline: 40 bytes */
>> };
>>
>> Can you elaborate on why this works for mac80211? It uses cb
>> only up to that point from where you invoke the prog?
>
> +1
>
> also didn't we discuss that wifi has crazy non-linear skb?
> this data/data_end is used by cls_bpf with headlen only
> for direct packet access where performance matters.
bpf_skb_pull_data() helper can be used as an option to pull
in more, though, f.e. up to bpf_skb_pull_data(skb, skb->len)
in the worst case, which then results in a fully linearized
skb where data/data_end has complete access. That much may
not be needed, though, but f.e. cls_bpf can certainly expand
the available headlen for direct packet access.
> Since wifi skbs have only eth in headlen, there is not much
> pointing adding support for data/data_end to wifi.
> Just use ld_abs/ld_ind instructions and load_bytes() helper.
Afaik, the ld_abs/ld_ind are not an option due to the data
on the wire being in little endian, but the bpf_skb_load_bytes()
might be the way to go initially, agree.
^ permalink raw reply
* [PATCH next] bonding: fix wq initialization for links created via netlink
From: Mahesh Bandewar @ 2017-04-20 0:30 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, Eric Dumazet
Cc: netdev, Mahesh Bandewar, Mahesh Bandewar
From: Mahesh Bandewar <maheshb@google.com>
Earlier patch 4493b81bea ("bonding: initialize work-queues during
creation of bond") moved the work-queue initialization from bond_open()
to bond_create(). However this caused the link those are created using
netlink 'create bond option' (ip link add bondX type bond); create the
new trunk without initializing work-queues. Prior to the above mentioned
change, ndo_open was in both paths and things worked correctly. The
consequence is visible in the report shared by Joe Stringer -
I've noticed that this patch breaks bonding within namespaces if
you're not careful to perform device cleanup correctly.
Here's my repro script, you can run on any net-next with this patch
and you'll start seeing some weird behaviour:
ip netns add foo
ip li add veth0 type veth peer name veth0+ netns foo
ip li add veth1 type veth peer name veth1+ netns foo
ip netns exec foo ip li add bond0 type bond
ip netns exec foo ip li set dev veth0+ master bond0
ip netns exec foo ip li set dev veth1+ master bond0
ip netns exec foo ip addr add dev bond0 192.168.0.1/24
ip netns exec foo ip li set dev bond0 up
ip li del dev veth0
ip li del dev veth1
The second to last command segfaults, last command hangs. rtnl is now
permanently locked. It's not a problem if you take bond0 down before
deleting veths, or delete bond0 before deleting veths. If you delete
either end of the veth pair as per above, either inside or outside the
namespace, it hits this problem.
Here's some kernel logs:
[ 1221.801610] bond0: Enslaving veth0+ as an active interface with an up link
[ 1224.449581] bond0: Enslaving veth1+ as an active interface with an up link
[ 1281.193863] bond0: Releasing backup interface veth0+
[ 1281.193866] bond0: the permanent HWaddr of veth0+ -
16:bf:fb:e0:b8:43 - is still in use by bond0 - set the HWaddr of
veth0+ to a different address to avoid conflicts
[ 1281.193867] ------------[ cut here ]------------
[ 1281.193873] WARNING: CPU: 0 PID: 2024 at kernel/workqueue.c:1511
__queue_delayed_work+0x13f/0x150
[ 1281.193873] Modules linked in: bonding veth openvswitch nf_nat_ipv6
nf_nat_ipv4 nf_nat autofs4 nfsd auth_rpcgss nfs_acl binfmt_misc nfs
lockd grace sunrpc fscache ppdev vmw_balloon coretemp psmouse
serio_raw vmwgfx ttm drm_kms_helper vmw_vmci netconsole parport_pc
configfs drm i2c_piix4 fb_sys_fops syscopyarea sysfillrect sysimgblt
shpchp mac_hid nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4
nf_defrag_ipv4 nf_conntrack libcrc32c lp parport hid_generic usbhid
hid mptspi mptscsih e1000 mptbase ahci libahci
[ 1281.193905] CPU: 0 PID: 2024 Comm: ip Tainted: G W
4.10.0-bisect-bond-v0.14 #37
[ 1281.193906] Hardware name: VMware, Inc. VMware Virtual
Platform/440BX Desktop Reference Platform, BIOS 6.00 09/30/2014
[ 1281.193906] Call Trace:
[ 1281.193912] dump_stack+0x63/0x89
[ 1281.193915] __warn+0xd1/0xf0
[ 1281.193917] warn_slowpath_null+0x1d/0x20
[ 1281.193918] __queue_delayed_work+0x13f/0x150
[ 1281.193920] queue_delayed_work_on+0x27/0x40
[ 1281.193929] bond_change_active_slave+0x25b/0x670 [bonding]
[ 1281.193932] ? synchronize_rcu_expedited+0x27/0x30
[ 1281.193935] __bond_release_one+0x489/0x510 [bonding]
[ 1281.193939] ? addrconf_notify+0x1b7/0xab0
[ 1281.193942] bond_netdev_event+0x2c5/0x2e0 [bonding]
[ 1281.193944] ? netconsole_netdev_event+0x124/0x190 [netconsole]
[ 1281.193947] notifier_call_chain+0x49/0x70
[ 1281.193948] raw_notifier_call_chain+0x16/0x20
[ 1281.193950] call_netdevice_notifiers_info+0x35/0x60
[ 1281.193951] rollback_registered_many+0x23b/0x3e0
[ 1281.193953] unregister_netdevice_many+0x24/0xd0
[ 1281.193955] rtnl_delete_link+0x3c/0x50
[ 1281.193956] rtnl_dellink+0x8d/0x1b0
[ 1281.193960] rtnetlink_rcv_msg+0x95/0x220
[ 1281.193962] ? __kmalloc_node_track_caller+0x35/0x280
[ 1281.193964] ? __netlink_lookup+0xf1/0x110
[ 1281.193966] ? rtnl_newlink+0x830/0x830
[ 1281.193967] netlink_rcv_skb+0xa7/0xc0
[ 1281.193969] rtnetlink_rcv+0x28/0x30
[ 1281.193970] netlink_unicast+0x15b/0x210
[ 1281.193971] netlink_sendmsg+0x319/0x390
[ 1281.193974] sock_sendmsg+0x38/0x50
[ 1281.193975] ___sys_sendmsg+0x25c/0x270
[ 1281.193978] ? mem_cgroup_commit_charge+0x76/0xf0
[ 1281.193981] ? page_add_new_anon_rmap+0x89/0xc0
[ 1281.193984] ? lru_cache_add_active_or_unevictable+0x35/0xb0
[ 1281.193985] ? __handle_mm_fault+0x4e9/0x1170
[ 1281.193987] __sys_sendmsg+0x45/0x80
[ 1281.193989] SyS_sendmsg+0x12/0x20
[ 1281.193991] do_syscall_64+0x6e/0x180
[ 1281.193993] entry_SYSCALL64_slow_path+0x25/0x25
[ 1281.193995] RIP: 0033:0x7f6ec122f5a0
[ 1281.193995] RSP: 002b:00007ffe69e89c48 EFLAGS: 00000246 ORIG_RAX:
000000000000002e
[ 1281.193997] RAX: ffffffffffffffda RBX: 00007ffe69e8dd60 RCX: 00007f6ec122f5a0
[ 1281.193997] RDX: 0000000000000000 RSI: 00007ffe69e89c90 RDI: 0000000000000003
[ 1281.193998] RBP: 00007ffe69e89c90 R08: 0000000000000000 R09: 0000000000000003
[ 1281.193999] R10: 00007ffe69e89a10 R11: 0000000000000246 R12: 0000000058f14b9f
[ 1281.193999] R13: 0000000000000000 R14: 00000000006473a0 R15: 00007ffe69e8e450
[ 1281.194001] ---[ end trace 713a77486cbfbfa3 ]---
Fixes: 4493b81bea ("bonding: initialize work-queues during creation of bond")
Reported-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
drivers/net/bonding/bond_main.c | 2 +-
drivers/net/bonding/bond_netlink.c | 5 +++++
include/net/bonding.h | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6bd3b50faf48..e549bf6f5cac 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3243,7 +3243,7 @@ u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb)
/*-------------------------- Device entry points ----------------------------*/
-static void bond_work_init_all(struct bonding *bond)
+void bond_work_init_all(struct bonding *bond)
{
INIT_DELAYED_WORK(&bond->mcast_work,
bond_resend_igmp_join_requests_delayed);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index b8df0f5e8c25..31b9f37c7dbd 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -449,6 +449,11 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
err = register_netdevice(bond_dev);
netif_carrier_off(bond_dev);
+ if (err >= 0) {
+ struct bonding *bond = netdev_priv(bond_dev);
+
+ bond_work_init_all(bond);
+ }
return err;
}
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 04a21e8048be..b00508d22e0a 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -614,6 +614,7 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
int level);
int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
+void bond_work_init_all(struct bonding *bond);
#ifdef CONFIG_PROC_FS
void bond_create_proc_entry(struct bonding *bond);
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* Re: [PATCH v4 net-next RFC] net: Generic XDP
From: Alexei Starovoitov @ 2017-04-20 0:13 UTC (permalink / raw)
To: Andy Gospodarek
Cc: John Fastabend, David Miller, michael.chan, netdev, xdp-newbies
In-Reply-To: <20170419202543.GL4730@C02RW35GFVH8.dhcp.broadcom.net>
On Wed, Apr 19, 2017 at 04:25:43PM -0400, Andy Gospodarek wrote:
> On Wed, Apr 19, 2017 at 10:44:59AM -0700, John Fastabend wrote:
> > On 17-04-19 10:17 AM, Alexei Starovoitov wrote:
> > > On Wed, Apr 19, 2017 at 10:29:03AM -0400, Andy Gospodarek wrote:
> > >>
> > >> I ran this on top of a card that uses the bnxt_en driver on a desktop
> > >> class system with an i7-6700 CPU @ 3.40GHz, sending a single stream of
> > >> UDP traffic with flow control disabled and saw the following (all stats
> > >> in Million PPS).
> > >>
> > >> xdp1 xdp2 xdp_tx_tunnel
> > >> Generic XDP 7.8 5.5 (1.3 actual) 4.6 (1.1 actual)
> > >> Optimized XDP 11.7 9.7 4.6
> > >
> > > Nice! Thanks for testing.
> > >
> > >> One thing to note is that the Generic XDP case shows some different
> > >> results for reported by the application vs actual (seen on the wire). I
> > >> did not debug where the drops are happening and what counter needs to be
> > >> incremented to note this -- I'll add that to my TODO list. The
> > >> Optimized XDP case does not have a difference in reported vs actual
> > >> frames on the wire.
> > >
> > > The missed packets are probably due to xmit queue being full.
> > > We need 'xdp_tx_full' counter in:
> > > + if (free_skb) {
> > > + trace_xdp_exception(dev, xdp_prog, XDP_TX);
> > > + kfree_skb(skb);
> > > + }
> > > like in-driver xdp does.
> > > It's surprising that tx becomes full so often. May be bnxt specific behavior?
> >
> > hmm as a data point I get better numbers than 1.3Mpps running through the qdisc
> > layer with pktgen so seems like something is wrong with the driver perhaps? If
>
> I get ~6.5Mpps on a single core with pktgen, so inconclusive for now....
may be your tx queue is simply smaller than rx queue?
^ permalink raw reply
* Re: __sk_buff.data_end
From: Alexei Starovoitov @ 2017-04-20 0:12 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Johannes Berg, Alexei Starovoitov, netdev
In-Reply-To: <58F7FA6D.5030000@iogearbox.net>
On Thu, Apr 20, 2017 at 02:01:49AM +0200, Daniel Borkmann wrote:
> On 04/20/2017 12:20 AM, Johannes Berg wrote:
> >On Wed, 2017-04-19 at 23:31 +0200, Johannes Berg wrote:
> >>Hi Alexei, Daniel,
> >>
> >>I'm looking at adding the __wifi_sk_buff I talked about, and I notice
> >>that it uses CB space to store data_end. Unfortunately, in a lot of
> >>cases, we don't have any CB space to spare in wifi.
> >
> >I guess I can work around this, would this seem reasonable?
> >
> > struct bpf_skb_data_end {
> > struct qdisc_skb_cb qdisc_cb;
> >- void *data_end;
> >+ /*
> >+ * The alignment here is for mac80211, since that doesn't use
> >+ * a pointer but a u64 value and needs to save/restore that
> >+ * across running its BPF programs.
> >+ */
> >+ void *data_end __aligned(sizeof(u64));
> > };
>
> Yeah, should work as well for the 32 bit archs, on 64 bit we
> have this effectively already:
>
> struct bpf_skb_data_end {
> struct qdisc_skb_cb qdisc_cb; /* 0 28 */
>
> /* XXX 4 bytes hole, try to pack */
>
> void * data_end; /* 32 8 */
>
> /* size: 40, cachelines: 1, members: 2 */
> /* sum members: 36, holes: 1, sum holes: 4 */
> /* last cacheline: 40 bytes */
> };
>
> Can you elaborate on why this works for mac80211? It uses cb
> only up to that point from where you invoke the prog?
+1
also didn't we discuss that wifi has crazy non-linear skb?
this data/data_end is used by cls_bpf with headlen only
for direct packet access where performance matters.
Since wifi skbs have only eth in headlen, there is not much
pointing adding support for data/data_end to wifi.
Just use ld_abs/ld_ind instructions and load_bytes() helper.
^ permalink raw reply
* Re: __sk_buff.data_end
From: Daniel Borkmann @ 2017-04-20 0:01 UTC (permalink / raw)
To: Johannes Berg, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <1492640459.22185.7.camel@sipsolutions.net>
On 04/20/2017 12:20 AM, Johannes Berg wrote:
> On Wed, 2017-04-19 at 23:31 +0200, Johannes Berg wrote:
>> Hi Alexei, Daniel,
>>
>> I'm looking at adding the __wifi_sk_buff I talked about, and I notice
>> that it uses CB space to store data_end. Unfortunately, in a lot of
>> cases, we don't have any CB space to spare in wifi.
>
> I guess I can work around this, would this seem reasonable?
>
> struct bpf_skb_data_end {
> struct qdisc_skb_cb qdisc_cb;
> - void *data_end;
> + /*
> + * The alignment here is for mac80211, since that doesn't use
> + * a pointer but a u64 value and needs to save/restore that
> + * across running its BPF programs.
> + */
> + void *data_end __aligned(sizeof(u64));
> };
Yeah, should work as well for the 32 bit archs, on 64 bit we
have this effectively already:
struct bpf_skb_data_end {
struct qdisc_skb_cb qdisc_cb; /* 0 28 */
/* XXX 4 bytes hole, try to pack */
void * data_end; /* 32 8 */
/* size: 40, cachelines: 1, members: 2 */
/* sum members: 36, holes: 1, sum holes: 4 */
/* last cacheline: 40 bytes */
};
Can you elaborate on why this works for mac80211? It uses cb
only up to that point from where you invoke the prog?
^ permalink raw reply
* Re: [kernel-hardening] Re: [PATCH net-next v6 04/11] landlock: Add LSM hooks related to filesystem
From: Casey Schaufler @ 2017-04-19 23:58 UTC (permalink / raw)
To: Mickaël Salaün, Kees Cook
Cc: LKML, Alexei Starovoitov, Andy Lutomirski,
Arnaldo Carvalho de Melo, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
Jonathan Corbet, Matthew Garrett, Michael Kerrisk, Paul Moore,
Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Tejun Heo,
Thomas Graf, Will Drewry, "kernel-hardening@lists.o
In-Reply-To: <35272f2b-ec5f-d032-ae2e-9fc0b4c0e2e3-WFhQfpSGs3bR7s880joybQ@public.gmane.org>
On 4/19/2017 3:03 PM, Mickaël Salaün wrote:
> On 19/04/2017 01:40, Kees Cook wrote:
>> On Tue, Apr 18, 2017 at 4:16 PM, Casey Schaufler <casey-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org> wrote:
>>> On 4/18/2017 3:44 PM, Mickaël Salaün wrote:
>>>> On 19/04/2017 00:17, Kees Cook wrote:
>>>>> On Tue, Mar 28, 2017 at 4:46 PM, Mickaël Salaün <mic-WFhQfpSGs3bR7s880joybQ@public.gmane.org> wrote:
>>>>>> +void __init landlock_add_hooks(void)
>>>>>> +{
>>>>>> + pr_info("landlock: Version %u", LANDLOCK_VERSION);
>>>>>> + landlock_add_hooks_fs();
>>>>>> + security_add_hooks(NULL, 0, "landlock");
>>>>>> + bpf_register_prog_type(&bpf_landlock_type);
>>>>> I'm confused by the separation of hook registration here. The call to
>>>>> security_add_hooks is with count=0 is especially weird. Why isn't this
>>>>> just a single call with security_add_hooks(landlock_hooks,
>>>>> ARRAY_SIZE(landlock_hooks), "landlock")?
>>>> Yes, this is ugly with the new security_add_hooks() with three arguments
>>>> but I wanted to split the hooks definition in multiple files.
>>> Why? I'll buy a good argument, but there are dangers in
>>> allowing multiple calls to security_add_hooks().
> I prefer to have one file per hook "family" (e.g. filesystem, network,
> ptrace…). This reduce the mess with all the included files (needed for
> LSM hook argument types) and make the files easier to read, understand
> and maintain.
Yeah, there's that tradeoff and it really is a matter
of taste I suppose.
>>>> The current security_add_hooks() use lsm_append(lsm, &lsm_names) which
>>>> is not exported. Unfortunately, calling multiple security_add_hooks()
>>>> with the same LSM name would register multiple names for the same LSM…
>>>> Is it OK if I modify this function to not add duplicated entries?
>>> It may seem absurd, but it's conceivable that a module might
>>> have two hooks it wants called. My example is a module that
>>> counts the number of times SELinux denies a process access to
>>> things (which needs to be called before and after SELinux in
>>> order to detect denials) and takes "appropriate action" if
>>> too many denials occur. It would be weird, wonky and hackish,
>>> but that never stopped anybody before.
> Right, but now, with the new lsm_append(), module names are concatenated
> ("%s,%s") in the lsm_names variable. It would be nice to not pollute
> this string with multiple time the same module name.
All it would take is a check that the module name
isn't already on the list. It's a trivial change.
>> If ends up being sane and clear, I'm fine with allowing multiple calls.
>>
>> -Kees
>>
^ permalink raw reply
* Re: __sk_buff.data_end
From: Daniel Borkmann @ 2017-04-19 23:51 UTC (permalink / raw)
To: Johannes Berg, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <1492637460.22185.6.camel@sipsolutions.net>
On 04/19/2017 11:31 PM, Johannes Berg wrote:
> Hi Alexei, Daniel,
>
> I'm looking at adding the __wifi_sk_buff I talked about, and I notice
> that it uses CB space to store data_end. Unfortunately, in a lot of
> cases, we don't have any CB space to spare in wifi.
>
> Is there any way to generate a series of instructions that instead does
> the necessary calculations? I don't actually *see* such a way, because
> I don't see how I could have a scratch register or scratch stack space,
> but perhaps there's a way to do it?
One option would be, similarly as in bpf_prog_run_save_cb(), to just
save / restore _only_ the data_end pointer portion for this. Calculating
this inline via bpf_convert_ctx_access() would indeed need one more
reg than just the dst reg available in order to perform the data_end
calculation only as BPF insns.
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: David Ahern @ 2017-04-19 23:51 UTC (permalink / raw)
To: Cong Wang, Andrey Konovalov
Cc: Dmitry Vyukov, Eric Dumazet, Mahesh Bandewar, Eric Dumazet,
David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, syzkaller
In-Reply-To: <CAM_iQpXyDN5GD+QLmWgJSc0bqV0ViK5LxSdwAV3XsHWr6vLJ2Q@mail.gmail.com>
On 4/19/17 5:47 PM, Cong Wang wrote:
> On Wed, Apr 19, 2017 at 9:12 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>
>> Anyway, I just finished simplifying the reproducer. Give this one a try.
>
> Thanks for providing such a minimal reproducer!
>
> The following patch could fix this crash, but I am not 100% sure if we should
> just clear these bits or reject them with an errno.
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 9db14189..cf524c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2086,7 +2086,7 @@ static struct rt6_info
> *ip6_route_info_create(struct fib6_config *cfg)
> } else
> rt->rt6i_prefsrc.plen = 0;
>
> - rt->rt6i_flags = cfg->fc_flags;
> + rt->rt6i_flags = cfg->fc_flags & ~(RTF_PCPU | RTF_CACHE);
>
> install_route:
> rt->dst.dev = dev;
>
I sent a patch returning EINVAL if RTF_PCPU is set in fc_flags
^ permalink raw reply
* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: Cong Wang @ 2017-04-19 23:47 UTC (permalink / raw)
To: Andrey Konovalov
Cc: David Ahern, Dmitry Vyukov, Eric Dumazet, Mahesh Bandewar,
Eric Dumazet, David Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, syzkaller
In-Reply-To: <CAAeHK+zZb=K4X9=NmSWQJL8bpgMbXoG+h4RasAyYDr+C5pkVYA@mail.gmail.com>
On Wed, Apr 19, 2017 at 9:12 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>
> Anyway, I just finished simplifying the reproducer. Give this one a try.
Thanks for providing such a minimal reproducer!
The following patch could fix this crash, but I am not 100% sure if we should
just clear these bits or reject them with an errno.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9db14189..cf524c2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2086,7 +2086,7 @@ static struct rt6_info
*ip6_route_info_create(struct fib6_config *cfg)
} else
rt->rt6i_prefsrc.plen = 0;
- rt->rt6i_flags = cfg->fc_flags;
+ rt->rt6i_flags = cfg->fc_flags & ~(RTF_PCPU | RTF_CACHE);
install_route:
rt->dst.dev = dev;
^ permalink raw reply related
* Re: [PATCH v4 04/18] dt-bindings: syscon: Add DT bindings documentation for Allwinner syscon
From: André Przywara @ 2017-04-19 23:38 UTC (permalink / raw)
To: Corentin Labbe, robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue
Cc: devicetree, netdev, linux-kernel, linux-sunxi, linux-arm-kernel
In-Reply-To: <20170412111400.2296-5-clabbe.montjoie@gmail.com>
On 12/04/17 12:13, Corentin Labbe wrote:
> This patch adds documentation for Device-Tree bindings for the
> syscon present in allwinner devices.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../devicetree/bindings/misc/allwinner,syscon.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/allwinner,syscon.txt
>
> diff --git a/Documentation/devicetree/bindings/misc/allwinner,syscon.txt b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> new file mode 100644
> index 0000000..c056c5b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> @@ -0,0 +1,19 @@
> +* Allwinner sun8i system controller
> +
> +This file describes the bindings for the system controller present in
> +Allwinner SoC H3, A83T and A64.
> +The principal function of this syscon is to control EMAC PHY choice and
> +config.
> +
> +Required properties for the system controller:
> +- reg: address and length of the register for the device.
> +- compatible: should be "syscon" and one of the following string:
> + "allwinner,sun8i-h3-system-controller"
> + "allwinner,sun8i-a64-system-controller"
While sun8i might make some sense technically, all 64-bit sunxi
compatible strings use the sun50i prefix to follow the Allwinner naming.
So this should read:
"allwinner,sun50i-a64-system-controller"
Also I am wondering if we should add a compatible string for the H5
(support for that SoC is in -next already):
"allwinner,sun50i-h5-system-controller"
Cheers,
Andre.
> + "allwinner,sun8i-a83t-system-controller"
> +
> +Example:
> +syscon: syscon@01c00000 {
> + compatible = "allwinner,sun8i-h3-system-controller", "syscon";
> + reg = <0x01c00000 0x1000>;
> +};
>
^ permalink raw reply
* Re: [RFC PATCH net] net/mlx5e: Race between mlx5e_update_stats() and getting the stats
From: Eric Dumazet @ 2017-04-19 23:35 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, Saeed Mahameed, kernel-team
In-Reply-To: <20170419215338.vrbvzkceufktatoo@giedrius-mbp.dhcp.thefacebook.com>
On Wed, 2017-04-19 at 14:53 -0700, Martin KaFai Lau wrote:
> Right, a temp and a memcpy should be enough to solve our spike problem.
> It may be the right fix for net.
>
> Agree that using a spinlock is better (likely changing state_lock
> to spinlock). A quick grep shows 80 line changes. Saeed, thoughts?
I was not advising replacing the mutex (maybe it is a mutex for good
reason), I simply suggested to use another spinlock only for this very
specific section.
Something like :
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index dc52053128bc752ccd398449330c24c0bdf8b3a1..9b2e1b79fded22d55e9409cb572308190679cfdd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -722,6 +722,7 @@ struct mlx5e_priv {
struct mlx5_core_dev *mdev;
struct net_device *netdev;
struct mlx5e_stats stats;
+ spinlock_t stats_lock;
struct mlx5e_tstamp tstamp;
u16 q_counter;
#ifdef CONFIG_MLX5_CORE_EN_DCB
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index a004a5a1a4c22a742ef3f9939769c6b5c9445f46..b4b7d43bf899cadca2c2a17151d35acac9773859 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -315,9 +315,11 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
mlx5e_update_stats(priv);
mutex_unlock(&priv->state_lock);
+ spin_lock(&priv->stats_lock);
for (i = 0; i < NUM_SW_COUNTERS; i++)
data[idx++] = MLX5E_READ_CTR64_CPU(&priv->stats.sw,
sw_stats_desc, i);
+ spin_unlock(&priv->stats_lock);
for (i = 0; i < MLX5E_NUM_Q_CNTRS(priv); i++)
data[idx++] = MLX5E_READ_CTR32_CPU(&priv->stats.qcnt,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 66c133757a5ee8daae122e93322306b1c5c44336..4d6672045b1126a8bab4d6f2035e6a9b830560d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -174,7 +174,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
{
- struct mlx5e_sw_stats *s = &priv->stats.sw;
+ struct mlx5e_sw_stats temp, *s = &temp;
struct mlx5e_rq_stats *rq_stats;
struct mlx5e_sq_stats *sq_stats;
u64 tx_offload_none = 0;
@@ -229,6 +229,9 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
s->link_down_events_phy = MLX5_GET(ppcnt_reg,
priv->stats.pport.phy_counters,
counter_set.phys_layer_cntrs.link_down_events);
+ spin_lock(&priv->stats_lock);
+ memcpy(&priv->stats.sw, s, sizeof(*s));
+ spin_unlock(&priv->stats_lock);
}
static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
@@ -2754,11 +2757,13 @@ mlx5e_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
stats->tx_packets = PPORT_802_3_GET(pstats, a_frames_transmitted_ok);
stats->tx_bytes = PPORT_802_3_GET(pstats, a_octets_transmitted_ok);
} else {
+ spin_lock(&priv->stats_lock);
stats->rx_packets = sstats->rx_packets;
stats->rx_bytes = sstats->rx_bytes;
stats->tx_packets = sstats->tx_packets;
stats->tx_bytes = sstats->tx_bytes;
stats->tx_dropped = sstats->tx_queue_dropped;
+ spin_unlock(&priv->stats_lock);
}
stats->rx_dropped = priv->stats.qcnt.rx_out_of_buffer;
@@ -3561,6 +3566,8 @@ static void mlx5e_build_nic_netdev_priv(struct mlx5_core_dev *mdev,
mutex_init(&priv->state_lock);
+ spin_lock_init(&priv->stats_lock);
+
INIT_WORK(&priv->update_carrier_work, mlx5e_update_carrier_work);
INIT_WORK(&priv->set_rx_mode_work, mlx5e_set_rx_mode_work);
INIT_WORK(&priv->tx_timeout_work, mlx5e_tx_timeout_work);
^ permalink raw reply related
* [PATCH net v2] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats
From: Martin KaFai Lau @ 2017-04-19 23:32 UTC (permalink / raw)
To: netdev; +Cc: Saeed Mahameed, Eric Dumazet, kernel-team
We have observed a sudden spike in rx/tx_packets and rx/tx_bytes
reported under /proc/net/dev. There is a race in mlx5e_update_stats()
and some of the get-stats functions (the one that we hit is the
mlx5e_get_stats() which is called by ndo_get_stats64()).
In particular, the very first thing mlx5e_update_sw_counters()
does is 'memset(s, 0, sizeof(*s))'. For example, if mlx5e_get_stats()
is unlucky at one point, rx_bytes and rx_packets could be 0. One second
later, a normal (and much bigger than 0) value will be reported.
This patch is to use a 'struct mlx5e_sw_stats temp' to avoid
a direct memset zero on priv->stats.sw.
mlx5e_update_vport_counters() has a similar race. Hence, addressed
together.
I am lucky enough to catch this 0-reset in rx multicast:
eth0: 41457665 76804 70 0 0 70 0 47085 15586634 87502 3 0 0 0 3 0
eth0: 41459860 76815 70 0 0 70 0 47094 15588376 87516 3 0 0 0 3 0
eth0: 41460577 76822 70 0 0 70 0 0 15589083 87521 3 0 0 0 3 0
eth0: 41463293 76838 70 0 0 70 0 47108 15595872 87538 3 0 0 0 3 0
eth0: 41463379 76839 70 0 0 70 0 47116 15596138 87539 3 0 0 0 3 0
Cc: Saeed Mahameed <saeedm@mellanox.com>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 66c133757a5e..246786bb861b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -174,7 +174,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
{
- struct mlx5e_sw_stats *s = &priv->stats.sw;
+ struct mlx5e_sw_stats temp, *s = &temp;
struct mlx5e_rq_stats *rq_stats;
struct mlx5e_sq_stats *sq_stats;
u64 tx_offload_none = 0;
@@ -229,12 +229,14 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
s->link_down_events_phy = MLX5_GET(ppcnt_reg,
priv->stats.pport.phy_counters,
counter_set.phys_layer_cntrs.link_down_events);
+ memcpy(&priv->stats.sw, s, sizeof(*s));
}
static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
{
+ struct mlx5e_vport_stats temp;
int outlen = MLX5_ST_SZ_BYTES(query_vport_counter_out);
- u32 *out = (u32 *)priv->stats.vport.query_vport_out;
+ u32 *out = (u32 *)temp.query_vport_out;
u32 in[MLX5_ST_SZ_DW(query_vport_counter_in)] = {0};
struct mlx5_core_dev *mdev = priv->mdev;
@@ -245,6 +247,7 @@ static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
memset(out, 0, outlen);
mlx5_cmd_exec(mdev, in, sizeof(in), out, outlen);
+ memcpy(priv->stats.vport.query_vport_out, out, outlen);
}
static void mlx5e_update_pport_counters(struct mlx5e_priv *priv)
--
2.9.3
^ permalink raw reply related
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