* [PATCH/v2] net: treewide: Fix typo found in DocBook/networking.xml
From: Masanari Iida @ 2014-09-04 14:44 UTC (permalink / raw)
To: linux-kernel, rdunlap, netdev, davem, linux-doc, trivial; +Cc: Masanari Iida
In-Reply-To: <20140903.203814.642492840095995090.davem@davemloft.net>
This patch fix spelling typo found in DocBook/networking.xml.
It is because the neworking.xml is generated from comments
in the source, I have to fix typo in comments within the source.
Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
---
include/linux/netdevice.h | 4 ++--
include/net/wimax.h | 2 +-
include/trace/events/irq.h | 4 ++--
net/core/datagram.c | 2 +-
net/core/gen_estimator.c | 2 +-
net/core/gen_stats.c | 2 +-
net/core/skbuff.c | 4 ++--
net/core/sock.c | 4 ++--
net/socket.c | 2 +-
9 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 039b237..d2b1704 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3249,7 +3249,7 @@ static inline int __dev_uc_sync(struct net_device *dev,
}
/**
- * __dev_uc_unsync - Remove synchonized addresses from device
+ * __dev_uc_unsync - Remove synchronized addresses from device
* @dev: device to sync
* @unsync: function to call if address should be removed
*
@@ -3293,7 +3293,7 @@ static inline int __dev_mc_sync(struct net_device *dev,
}
/**
- * __dev_mc_unsync - Remove synchonized addresses from device
+ * __dev_mc_unsync - Remove synchronized addresses from device
* @dev: device to sync
* @unsync: function to call if address should be removed
*
diff --git a/include/net/wimax.h b/include/net/wimax.h
index e52ef53..c52b685 100644
--- a/include/net/wimax.h
+++ b/include/net/wimax.h
@@ -290,7 +290,7 @@ struct wimax_dev;
* This operation has to be synchronous, and return only when the
* reset is complete. In case of having had to resort to bus/cold
* reset implying a device disconnection, the call is allowed to
- * return inmediately.
+ * return immediately.
* NOTE: wimax_dev->mutex is NOT locked when this op is being
* called; however, wimax_dev->mutex_reset IS locked to ensure
* serialization of calls to wimax_reset().
diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 1c09820..3608beb 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -107,7 +107,7 @@ DECLARE_EVENT_CLASS(softirq,
* @vec_nr: softirq vector number
*
* When used in combination with the softirq_exit tracepoint
- * we can determine the softirq handler runtine.
+ * we can determine the softirq handler routine.
*/
DEFINE_EVENT(softirq, softirq_entry,
@@ -121,7 +121,7 @@ DEFINE_EVENT(softirq, softirq_entry,
* @vec_nr: softirq vector number
*
* When used in combination with the softirq_entry tracepoint
- * we can determine the softirq handler runtine.
+ * we can determine the softirq handler routine.
*/
DEFINE_EVENT(softirq, softirq_exit,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 488dd1a..fdbc9a8 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -775,7 +775,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
EXPORT_SYMBOL(__skb_checksum_complete);
/**
- * skb_copy_and_csum_datagram_iovec - Copy and checkum skb to user iovec.
+ * skb_copy_and_csum_datagram_iovec - Copy and checksum skb to user iovec.
* @skb: skbuff
* @hlen: hardware length
* @iov: io vector
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 6b5b6e7..9d33dff 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -197,7 +197,7 @@ struct gen_estimator *gen_find_node(const struct gnet_stats_basic_packed *bstats
* as destination. A new timer with the interval specified in the
* configuration TLV is created. Upon each interval, the latest statistics
* will be read from &bstats and the estimated rate will be stored in
- * &rate_est with the statistics lock grabed during this period.
+ * &rate_est with the statistics lock grabbed during this period.
*
* Returns 0 on success or a negative error code.
*
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 9d3d9e7..2ddbce4 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -206,7 +206,7 @@ EXPORT_SYMBOL(gnet_stats_copy_queue);
* @st: application specific statistics data
* @len: length of data
*
- * Appends the application sepecific statistics to the top level TLV created by
+ * Appends the application specific statistics to the top level TLV created by
* gnet_stats_start_copy() and remembers the data for XSTATS if the dumping
* handle is in backward compatibility mode.
*
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index adfc7ee..2d9f117 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2647,7 +2647,7 @@ EXPORT_SYMBOL(skb_prepare_seq_read);
* skb_seq_read() will return the remaining part of the block.
*
* Note 1: The size of each block of data returned can be arbitrary,
- * this limitation is the cost for zerocopy seqeuental
+ * this limitation is the cost for zerocopy sequential
* reads of potentially non linear data.
*
* Note 2: Fragment lists within fragments are not implemented
@@ -2781,7 +2781,7 @@ EXPORT_SYMBOL(skb_find_text);
/**
* skb_append_datato_frags - append the user data to a skb
* @sk: sock structure
- * @skb: skb structure to be appened with user data.
+ * @skb: skb structure to be appended with user data.
* @getfrag: call back function to be used for getting the user data
* @from: pointer to user message iov
* @length: length of the iov message
diff --git a/net/core/sock.c b/net/core/sock.c
index f7f2352..8bde925 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -166,7 +166,7 @@ EXPORT_SYMBOL(sk_ns_capable);
/**
* sk_capable - Socket global capability test
* @sk: Socket to use a capability on or through
- * @cap: The global capbility to use
+ * @cap: The global capability to use
*
* Test to see if the opener of the socket had when the socket was
* created and the current process has the capability @cap in all user
@@ -183,7 +183,7 @@ EXPORT_SYMBOL(sk_capable);
* @sk: Socket to use a capability on or through
* @cap: The capability to use
*
- * Test to see if the opener of the socket had when the socke was created
+ * Test to see if the opener of the socket had when the socket was created
* and the current process has the capability @cap over the network namespace
* the socket is a member of.
*/
diff --git a/net/socket.c b/net/socket.c
index 95ee7d8..0593ebd 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2602,7 +2602,7 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
*
* This function is called by a protocol handler that wants to
* advertise its address family, and have it linked into the
- * socket interface. The value ops->family coresponds to the
+ * socket interface. The value ops->family corresponds to the
* socket system call protocol family.
*/
int sock_register(const struct net_proto_family *ops)
--
2.1.0.24.g4109c28
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 14:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Laight, Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev@vger.kernel.org,
Markos Chandras, Martin Schwidefsky
In-Reply-To: <1409841347.26422.118.camel@edumazet-glaptop2.roam.corp.google.com>
On Do, 2014-09-04 at 07:35 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 16:33 +0200, Hannes Frederic Sowa wrote:
>
> > Which btw. also uses int, which might change alignment of structures.
>
> You missed the point .
>
> kmemcheck wants to make sure the whole word is set, or else you could
> get false positives.
>
> kmemcheck needs are quite different.
Now that you said it, I understand. :)
You were right with the int vs. u8 thing all along. gcc aligns the
datatype on the next struct field and not on the whole field, as I
expected. So excuse my error above.
I think the latest proposals looks good?
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Eric Dumazet @ 2014-09-04 14:35 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Laight, Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev@vger.kernel.org,
Markos Chandras, Martin Schwidefsky
In-Reply-To: <1409841233.23465.22.camel@localhost>
On Thu, 2014-09-04 at 16:33 +0200, Hannes Frederic Sowa wrote:
> Which btw. also uses int, which might change alignment of structures.
You missed the point .
kmemcheck wants to make sure the whole word is set, or else you could
get false positives.
kmemcheck needs are quite different.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 14:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Laight, Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev@vger.kernel.org,
Markos Chandras, Martin Schwidefsky
In-Reply-To: <1409841129.26422.117.camel@edumazet-glaptop2.roam.corp.google.com>
On Do, 2014-09-04 at 07:32 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 14:23 +0000, David Laight wrote:
> > From: Eric Dumazet
> > > On Thu, 2014-09-04 at 15:40 +0200, Hannes Frederic Sowa wrote:
> > >
> > > > The type does not matter at all. Actually I wanted to use an empty
> > > > struct but was afraid it might not work on older compilers and didn't
> > > > want to check that with each version.
> > >
> > >
> > > It matters. Really.
> > >
> > > $ cat try.c
> > > #include <stdio.h>
> > >
> > > struct S_int {
> > > char a;
> > > int offset[0];
> > > char b:1;
> > > };
> > >
> > > struct S_char {
> > > char a;
> > > char offset[0];
> > > char b:1;
> > > };
> >
> > Those are also both illegal C, zero sized arrays aren't allowed.
> >
> > David
>
>
> This does not matter, we already use such constructs in the kernel.
>
> Take a look at kmemcheck_bitfield_begin() for example.
Which btw. also uses int, which might change alignment of structures.
^ permalink raw reply
* [PATCH net-next] be2net: Learn and program mac to avoid packet replication in nPAR mode.
From: Venkat Duvvuru @ 2014-09-04 14:30 UTC (permalink / raw)
To: netdev; +Cc: Venkat Duvvuru
In a multi-channel setup, when an interface (channel/partition) is used
by a bridge or ovs, it is placed in promiscuous mode and the MAC addresses
of the VMs attached to the bridge are not configured on the base interface.
As a result of that, when a packet arrives to the port with
virtual machine's mac address, the card cannot determine which ring to
send the packet to, so replicates the packet on all the PFs of that port,
hence resulting in wastage of PCI bandwidth and CPU cycles. Packet replication
is also considered security risk as it can cause packets to reach an undesired VM.
This patch will help solve the problem by learning the mac address and
programming it in the adapter. This patch also unlearns the MAC, if the MAC is
moved out of the machine or if the MAC is inactive for more than 5 minutes.
Signed-off-by: Venkat Duvvuru <VenkatKumar.Duvvuru@Emulex.com>
---
drivers/net/ethernet/emulex/benet/be.h | 45 ++++
drivers/net/ethernet/emulex/benet/be_cmds.c | 27 +++
drivers/net/ethernet/emulex/benet/be_cmds.h | 31 +++-
drivers/net/ethernet/emulex/benet/be_main.c | 336 ++++++++++++++++++++++++++-
4 files changed, 437 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index a9f239a..9862ada 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -388,6 +388,9 @@ enum vf_state {
#define BE_FLAGS_QNQ_ASYNC_EVT_RCVD (1 << 11)
#define BE_FLAGS_VXLAN_OFFLOADS (1 << 12)
#define BE_FLAGS_SETUP_DONE (1 << 13)
+#define BE_FLAGS_MAC_LEARNING_INITIALIZED (1 << 14)
+#define BE_FLAGS_MAC_LEARNING_ENABLED (1 << 15)
+#define BE_FLAGS_MAC_FILTERS_EXHAUSTED (1 << 16)
#define BE_UC_PMAC_COUNT 30
#define BE_VF_UC_PMAC_COUNT 2
@@ -425,6 +428,45 @@ struct be_resources {
u32 vf_if_cap_flags; /* VF if capability flags */
};
+#define BE_MAC_AGE 300000 /* in msecs */
+#define BE_MAC_MISS 0
+#define BE_MAC_HIT 1
+#define BE_MAC_HASH_TABLE_LEN 64
+#define be_mac_matches(node, ethhdr) \
+ (node && \
+ ether_addr_equal_64bits(node->mac, \
+ ethhdr->h_source)) \
+
+#define be_mac_vlan_matches(node, ethhdr, vlan_tag) \
+ (node && \
+ ether_addr_equal_64bits(node->mac, \
+ ethhdr->h_source) && \
+ (node->vlan_tag == vlan_tag))
+
+#define be_is_mac_learning_enabled(adapter) \
+ ((adapter->flags & BE_FLAGS_MAC_LEARNING_INITIALIZED) && \
+ (adapter->flags & BE_FLAGS_MAC_LEARNING_ENABLED) && \
+ !(adapter->flags & BE_FLAGS_MAC_FILTERS_EXHAUSTED))
+
+struct be_config_work {
+ struct work_struct work;
+ union {
+ struct be_mac_node *mac;
+ } data;
+};
+
+struct be_mac_node {
+ struct rcu_head rcu;
+ struct hlist_node list;
+ struct be_adapter *adapter;
+ u32 pmac_id;
+ u8 mac[ETH_ALEN];
+ u16 vlan_tag;
+ unsigned long jiffies;
+ u8 hashidx;
+ bool del;
+};
+
struct rss_info {
u64 rss_flags;
u8 rsstable[RSS_INDIR_TABLE_LEN];
@@ -532,6 +574,9 @@ struct be_adapter {
int be_get_temp_freq;
u8 pf_number;
struct rss_info rss_info;
+ struct hlist_head *mac_table;
+ spinlock_t mac_hash_lock; /* For serializing mac addr hash entries */
+ struct workqueue_struct *config_workq;
};
#define be_physfn(adapter) (!adapter->virtfn)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 5be100d..ec09bdb 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -292,6 +292,30 @@ static void be_async_grp5_pvid_state_process(struct be_adapter *adapter,
}
}
+static void be_async_grp5_fw_control_process(struct be_adapter *adapter,
+ struct be_mcc_compl *compl)
+{
+ struct be_async_fw_ctrl *evt = (struct be_async_fw_ctrl *)compl;
+ u8 learn_enable, learn_disable;
+
+ be_dws_le_to_cpu(&evt->context, sizeof(evt->context));
+
+ learn_enable = AMAP_GET_BITS(struct amap_be_async_fw_ctrl,
+ learn_enable, &evt->context);
+ learn_disable = AMAP_GET_BITS(struct amap_be_async_fw_ctrl,
+ learn_disable, &evt->context);
+
+ /* If both learn_enable and learn_disable are not set, then there's
+ * no change in learning state
+ */
+ if (learn_enable || learn_disable) {
+ if (learn_enable)
+ adapter->flags |= BE_FLAGS_MAC_LEARNING_ENABLED;
+ else
+ adapter->flags &= ~BE_FLAGS_MAC_LEARNING_ENABLED;
+ }
+}
+
static void be_async_grp5_evt_process(struct be_adapter *adapter,
struct be_mcc_compl *compl)
{
@@ -308,6 +332,9 @@ static void be_async_grp5_evt_process(struct be_adapter *adapter,
case ASYNC_EVENT_PVID_STATE:
be_async_grp5_pvid_state_process(adapter, compl);
break;
+ case ASYNC_EVENT_FW_CONTROL:
+ be_async_grp5_fw_control_process(adapter, compl);
+ break;
default:
dev_warn(&adapter->pdev->dev, "Unknown grp5 event 0x%x!\n",
event_type);
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 0e11868..6f98e55 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -99,6 +99,7 @@ struct be_mcc_compl {
#define ASYNC_EVENT_QOS_SPEED 0x1
#define ASYNC_EVENT_COS_PRIORITY 0x2
#define ASYNC_EVENT_PVID_STATE 0x3
+#define ASYNC_EVENT_FW_CONTROL 0x5
#define ASYNC_EVENT_CODE_QNQ 0x6
#define ASYNC_DEBUG_EVENT_TYPE_QNQ 1
@@ -168,6 +169,34 @@ struct be_async_event_qnq {
u32 flags;
} __packed;
+/* async event indicating bmc and mac learning */
+struct amap_be_async_fw_ctrl {
+ u8 learn_enable[1]; /* dword 0 */
+ u8 learn_disable[1]; /* dword 0 */
+ u8 mgmt_enable[1]; /* dword 0 */
+ u8 mgmt_disable[1]; /* dword 0 */
+ u8 rsvd0[12]; /* dword 0 */
+ u8 vlan_tag[16]; /* dword 0 */
+ u8 arp_filter[1]; /* dword 1 */
+ u8 dhcp_client_filt[1]; /* dword 1 */
+ u8 dhcp_server_filt[1]; /* dword 1 */
+ u8 net_bios_filt[1]; /* dword 1 */
+ u8 rsvd1[3]; /* dword 1 */
+ u8 bcast_filt[1]; /* dword 1 */
+ u8 ipv6_nbr_filt[1]; /* dword 1 */
+ u8 ipv6_ra_filt[1]; /* dword 1 */
+ u8 ipv6_ras_filt[1]; /* dword 1 */
+ u8 rsvd2[4]; /* dword 1 */
+ u8 mcast_filt[1]; /* dword 1 */
+ u8 rsvd3[16]; /* dword 1 */
+ u8 evt_tag[32]; /* dword 2 */
+ u8 dword3[32]; /* dword 3 */
+} __packed;
+
+struct be_async_fw_ctrl {
+ u8 context[sizeof(struct amap_be_async_fw_ctrl) / 8];
+};
+
struct be_mcc_mailbox {
struct be_mcc_wrb wrb;
struct be_mcc_compl compl;
@@ -1907,7 +1936,7 @@ enum mc_type {
UMC = 0x02,
FLEX10 = 0x03,
vNIC1 = 0x04,
- nPAR = 0x05,
+ NPAR = 0x05,
UFP = 0x06,
vNIC2 = 0x07
};
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 5b26c4c..b695acc 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -701,6 +701,21 @@ static inline void wrb_fill(struct be_eth_wrb *wrb, u64 addr, int len)
wrb->rsvd0 = 0;
}
+static u16 be_get_vlan(struct be_adapter *adapter,
+ struct sk_buff *skb)
+{
+ struct vlan_ethhdr *veh = vlan_eth_hdr(skb);
+ u16 vlan_id = 0;
+
+ /* inline vlan */
+ if (veh->h_vlan_proto == htons(ETH_P_8021Q))
+ vlan_id = ntohs(veh->h_vlan_TCI) & VLAN_VID_MASK;
+ else
+ vlan_id = vlan_tx_tag_get_id(skb);
+
+ return vlan_id;
+}
+
static inline u16 be_get_tx_vlan_tag(struct be_adapter *adapter,
struct sk_buff *skb)
{
@@ -998,6 +1013,249 @@ err:
return NULL;
}
+/* Mac hash function */
+static u8 be_mac_hash(u8 mac_addr[])
+{
+ return (mac_addr[5] & 7) |
+ ((mac_addr[2] & 3) << 3) |
+ ((mac_addr[1] & 1) << 5);
+}
+
+static void be_free_mac_node_rcu(struct rcu_head *head)
+{
+ struct be_mac_node *node = container_of(head, struct be_mac_node, rcu);
+
+ kfree(node);
+}
+
+static void be_del_mac_node_rcu(struct be_adapter *adapter,
+ struct be_mac_node *node)
+{
+ spin_lock_bh(&adapter->mac_hash_lock);
+ hlist_del_rcu(&node->list);
+ call_rcu_bh(&node->rcu, be_free_mac_node_rcu);
+ spin_unlock_bh(&adapter->mac_hash_lock);
+}
+
+/* Worker function to unlearn macs that are learnt */
+static void be_del_mac(struct work_struct *work)
+{
+ struct be_config_work *cfg_work = container_of(work,
+ struct be_config_work,
+ work);
+ struct be_mac_node *node = cfg_work->data.mac;
+ struct be_adapter *adapter = node->adapter;
+ struct hlist_head *head;
+ int status;
+
+ status = be_cmd_pmac_del(adapter,
+ adapter->if_handle,
+ node->pmac_id, 0);
+ if (!status) {
+ adapter->flags &= ~BE_FLAGS_MAC_FILTERS_EXHAUSTED;
+ kfree(node);
+ } else {
+ head = &adapter->mac_table[node->hashidx];
+ spin_lock_bh(&adapter->mac_hash_lock);
+ hlist_add_head_rcu(&node->list, head);
+ spin_unlock_bh(&adapter->mac_hash_lock);
+ }
+ kfree(work);
+}
+
+/* Worker function to program macs that are learnt */
+static void be_add_mac(struct work_struct *work)
+{
+ struct be_config_work *cfg_work = container_of(work,
+ struct be_config_work,
+ work);
+ struct be_mac_node *node = cfg_work->data.mac;
+ struct be_adapter *adapter = node->adapter;
+ int status;
+
+ status = be_cmd_pmac_add(adapter, (u8 *)node->mac,
+ adapter->if_handle,
+ &node->pmac_id, 0);
+
+ if (addl_status(status) == MCC_ADDL_STATUS_INSUFFICIENT_RESOURCES)
+ adapter->flags |= BE_FLAGS_MAC_FILTERS_EXHAUSTED;
+ else if (status)
+ be_del_mac_node_rcu(adapter, node);
+
+ kfree(work);
+}
+
+static void be_work_del_mac(struct rcu_head *rhead)
+{
+ struct be_mac_node *node = container_of(rhead, struct be_mac_node, rcu);
+ struct be_adapter *adapter = node->adapter;
+ struct be_config_work *work = NULL;
+ struct hlist_head *head;
+
+ work = kzalloc(sizeof(*work), GFP_ATOMIC);
+ if (!work) {
+ head = &adapter->mac_table[node->hashidx];
+ spin_lock_bh(&adapter->mac_hash_lock);
+ hlist_add_head_rcu(&node->list, head);
+ spin_unlock_bh(&adapter->mac_hash_lock);
+ return;
+ }
+
+ work->data.mac = node;
+
+ INIT_WORK(&work->work, be_del_mac);
+ queue_work(node->adapter->config_workq, &work->work);
+}
+
+/* Keep checking each mac's age for every second and unlearn mac if not
+ * active for more than 5 minutes.
+ */
+static void be_check_mac_aging(struct be_adapter *adapter)
+{
+ struct be_mac_node *node = NULL;
+ unsigned int age_msecs, i;
+ struct hlist_head *head;
+ unsigned long curr_time;
+
+ rcu_read_lock_bh();
+ for (i = 0; i < BE_MAC_HASH_TABLE_LEN; i++) {
+ head = &adapter->mac_table[i];
+ hlist_for_each_entry_rcu_bh(node, head, list) {
+ curr_time = jiffies;
+ if (time_before(curr_time, node->jiffies))
+ continue;
+ age_msecs = jiffies_to_msecs(curr_time - node->jiffies);
+ if (age_msecs >= BE_MAC_AGE) {
+ spin_lock_bh(&adapter->mac_hash_lock);
+ /* Unlearn also can delete this node */
+ if (!node->del) {
+ hlist_del_rcu(&node->list);
+ node->del = true;
+ call_rcu_bh(&node->rcu,
+ be_work_del_mac);
+ }
+ spin_unlock_bh(&adapter->mac_hash_lock);
+ }
+ }
+ }
+ rcu_read_unlock_bh();
+}
+
+static void be_unlearn_mac(struct be_adapter *adapter,
+ struct sk_buff *skb)
+{
+ struct ethhdr *ethhdr = eth_hdr(skb);
+ struct be_mac_node *node = NULL;
+ struct hlist_head *head;
+ u16 vlan_tag = 0;
+ u8 hashidx = 0;
+
+ vlan_tag = be_get_vlan(adapter, skb);
+ hashidx = be_mac_hash(ethhdr->h_source);
+ head = &adapter->mac_table[hashidx];
+
+ rcu_read_lock_bh();
+ hlist_for_each_entry_rcu_bh(node, head, list) {
+ if (be_mac_vlan_matches(node, ethhdr, vlan_tag)) {
+ spin_lock_bh(&adapter->mac_hash_lock);
+ /* Aging also can delete this node */
+ if (!node->del) {
+ hlist_del_rcu(&node->list);
+ node->del = true;
+ call_rcu_bh(&node->rcu, be_work_del_mac);
+ }
+ spin_unlock_bh(&adapter->mac_hash_lock);
+ break;
+ }
+ }
+ rcu_read_unlock_bh();
+}
+
+static int be_work_add_mac(struct be_adapter *adapter,
+ struct be_mac_node *node)
+{
+ struct be_config_work *mac_work = NULL;
+
+ mac_work = kzalloc(sizeof(*mac_work), GFP_ATOMIC);
+ if (!mac_work)
+ return -ENOMEM;
+ mac_work->data.mac = node;
+
+ INIT_WORK(&mac_work->work, be_add_mac);
+ queue_work(adapter->config_workq, &mac_work->work);
+ return 0;
+}
+
+static void be_check_add_mac_node(struct be_adapter *adapter,
+ struct sk_buff *skb,
+ u8 hashidx)
+{
+ struct hlist_head *head = &adapter->mac_table[hashidx];
+ struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
+ struct be_mac_node *node = NULL;
+ struct be_mac_node *tmp = NULL;
+ u16 vlan_tag = 0;
+ int status;
+
+ vlan_tag = be_get_vlan(adapter, skb);
+
+ rcu_read_lock_bh();
+ hlist_for_each_entry_rcu_bh(node, head, list) {
+ if (be_mac_matches(node, ethhdr)) {
+ node->jiffies = jiffies;
+ rcu_read_unlock_bh();
+ return; /* Mac found in the table */
+ }
+ }
+ rcu_read_unlock_bh();
+
+ /* Mac not found. Allocate a new node and add it to the table */
+ node = kzalloc(sizeof(*node), GFP_ATOMIC);
+ if (!node)
+ return;
+ ether_addr_copy(node->mac, ethhdr->h_source);
+ node->jiffies = jiffies;
+ node->vlan_tag = vlan_tag;
+ node->adapter = adapter;
+ node->hashidx = hashidx;
+
+ spin_lock_bh(&adapter->mac_hash_lock);
+ hlist_for_each_entry_rcu_bh(tmp, head, list) {
+ if (be_mac_matches(tmp, ethhdr)) {
+ kfree(node);
+ spin_unlock_bh(&adapter->mac_hash_lock);
+ return; /* Already added */
+ }
+ }
+ hlist_add_head_rcu(&node->list, head);
+ spin_unlock_bh(&adapter->mac_hash_lock);
+
+ status = be_work_add_mac(adapter, node);
+ if (status)
+ be_del_mac_node_rcu(adapter, node);
+}
+
+/* When hypervisor OSes configure the port in promiscuous (MAC and VLAN)
+ * mode and do not program vNIC's VLAN/MAC in the adapter, when a packet arrives
+ * in the port with a vNIC's VLAN/MAC, the card cannot determine which ring to
+ * send the packet to, so sends the packet to all the PFs of that port.
+ * Learning mac and programming it in the adapter will solve the problem.
+ */
+static void be_learn_mac(struct be_adapter *adapter,
+ struct sk_buff *skb)
+{
+ struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
+ u8 hashidx;
+
+ if (ether_addr_equal_64bits(ethhdr->h_source,
+ adapter->netdev->dev_addr))
+ return;
+
+ hashidx = be_mac_hash(ethhdr->h_source);
+
+ be_check_add_mac_node(adapter, skb, hashidx);
+}
+
static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
struct sk_buff *skb,
bool *skip_hw_vlan)
@@ -1059,6 +1317,9 @@ static netdev_tx_t be_xmit(struct sk_buff *skb, struct net_device *netdev)
stopped = true;
}
+ if (be_is_mac_learning_enabled(adapter))
+ be_learn_mac(adapter, skb);
+
be_txq_notify(adapter, txo, wrb_cnt);
be_tx_stats_update(txo, wrb_cnt, copied, gso_segs, stopped);
@@ -1689,6 +1950,12 @@ static void be_rx_compl_process(struct be_rx_obj *rxo, struct napi_struct *napi,
if (rxcp->vlanf)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), rxcp->vlan_tag);
+ /* Unlearn the mac that is already learnt just in case
+ * the mac moves out of the machine.
+ */
+ if (adapter->flags & BE_FLAGS_MAC_LEARNING_INITIALIZED)
+ be_unlearn_mac(adapter, skb);
+
netif_receive_skb(skb);
}
@@ -3073,6 +3340,31 @@ done:
adapter->flags &= ~BE_FLAGS_SRIOV_ENABLED;
}
+static void be_clear_mac_learning(struct be_adapter *adapter)
+{
+ int j;
+ struct hlist_node *n;
+ struct hlist_head *head;
+ struct be_mac_node *node = NULL;
+
+ if (adapter->flags & BE_FLAGS_MAC_LEARNING_INITIALIZED) {
+ adapter->flags &= ~BE_FLAGS_MAC_LEARNING_INITIALIZED;
+ /* Flush all the pending rcu callbacks */
+ rcu_barrier();
+
+ for (j = 0; j < BE_MAC_HASH_TABLE_LEN; j++) {
+ head = &adapter->mac_table[j];
+ hlist_for_each_entry_safe(node, n, head, list) {
+ be_cmd_pmac_del(adapter,
+ adapter->if_handle,
+ node->pmac_id, 0);
+ kfree(node);
+ }
+ }
+ kfree(adapter->mac_table);
+ }
+}
+
static void be_clear_queues(struct be_adapter *adapter)
{
be_mcc_queues_destroy(adapter);
@@ -3121,6 +3413,14 @@ static void be_disable_vxlan_offloads(struct be_adapter *adapter)
static int be_clear(struct be_adapter *adapter)
{
+ if (adapter->config_workq) {
+ flush_workqueue(adapter->config_workq);
+ destroy_workqueue(adapter->config_workq);
+ }
+
+ if (adapter->mc_type == NPAR)
+ be_clear_mac_learning(adapter);
+
be_cancel_worker(adapter);
if (sriov_enabled(adapter))
@@ -3514,6 +3814,27 @@ static int be_get_config(struct be_adapter *adapter)
return 0;
}
+static void be_setup_mac_learning(struct be_adapter *adapter)
+{
+ int i;
+
+ adapter->mac_table = kcalloc(BE_MAC_HASH_TABLE_LEN,
+ sizeof(struct hlist_head),
+ GFP_KERNEL);
+ if (!adapter->mac_table) {
+ dev_err(&adapter->pdev->dev,
+ "Mac hash table alloc failed\n");
+ return;
+ }
+
+ spin_lock_init(&adapter->mac_hash_lock);
+
+ for (i = 0; i < BE_MAC_HASH_TABLE_LEN; i++)
+ INIT_HLIST_HEAD(&adapter->mac_table[i]);
+
+ adapter->flags |= BE_FLAGS_MAC_LEARNING_INITIALIZED;
+}
+
static int be_mac_setup(struct be_adapter *adapter)
{
u8 mac[ETH_ALEN];
@@ -3653,6 +3974,16 @@ static int be_setup(struct be_adapter *adapter)
be_cmd_get_fn_privileges(adapter, &adapter->cmd_privileges, 0);
+ adapter->config_workq = create_singlethread_workqueue("be_config");
+ if (!adapter->config_workq) {
+ status = -ENOMEM;
+ dev_err(dev, "Cannot create be_config workqueue\n");
+ goto err;
+ }
+
+ if (adapter->mc_type == NPAR)
+ be_setup_mac_learning(adapter);
+
status = be_mac_setup(adapter);
if (status)
goto err;
@@ -4784,6 +5115,9 @@ static void be_worker(struct work_struct *work)
be_eqd_update(adapter);
+ if (adapter->flags & BE_FLAGS_MAC_LEARNING_INITIALIZED)
+ be_check_mac_aging(adapter);
+
reschedule:
adapter->work_counter++;
schedule_delayed_work(&adapter->work, msecs_to_jiffies(1000));
@@ -4809,7 +5143,7 @@ static char *mc_name(struct be_adapter *adapter)
case vNIC1:
str = "vNIC-1";
break;
- case nPAR:
+ case NPAR:
str = "nPAR";
break;
case UFP:
--
1.7.1
^ permalink raw reply related
* RE: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Eric Dumazet @ 2014-09-04 14:32 UTC (permalink / raw)
To: David Laight
Cc: Hannes Frederic Sowa, Denis Kirjanov, Alexei Starovoitov,
Daniel Borkmann, Eric Dumazet, Denis Kirjanov,
netdev@vger.kernel.org, Markos Chandras, Martin Schwidefsky
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174876B4@AcuExch.aculab.com>
On Thu, 2014-09-04 at 14:23 +0000, David Laight wrote:
> From: Eric Dumazet
> > On Thu, 2014-09-04 at 15:40 +0200, Hannes Frederic Sowa wrote:
> >
> > > The type does not matter at all. Actually I wanted to use an empty
> > > struct but was afraid it might not work on older compilers and didn't
> > > want to check that with each version.
> >
> >
> > It matters. Really.
> >
> > $ cat try.c
> > #include <stdio.h>
> >
> > struct S_int {
> > char a;
> > int offset[0];
> > char b:1;
> > };
> >
> > struct S_char {
> > char a;
> > char offset[0];
> > char b:1;
> > };
>
> Those are also both illegal C, zero sized arrays aren't allowed.
>
> David
This does not matter, we already use such constructs in the kernel.
Take a look at kmemcheck_bitfield_begin() for example.
^ permalink raw reply
* RE: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: David Laight @ 2014-09-04 14:23 UTC (permalink / raw)
To: 'Eric Dumazet', Hannes Frederic Sowa
Cc: Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <1409839892.26422.115.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet
> On Thu, 2014-09-04 at 15:40 +0200, Hannes Frederic Sowa wrote:
>
> > The type does not matter at all. Actually I wanted to use an empty
> > struct but was afraid it might not work on older compilers and didn't
> > want to check that with each version.
>
>
> It matters. Really.
>
> $ cat try.c
> #include <stdio.h>
>
> struct S_int {
> char a;
> int offset[0];
> char b:1;
> };
>
> struct S_char {
> char a;
> char offset[0];
> char b:1;
> };
Those are also both illegal C, zero sized arrays aren't allowed.
David
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 14:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <1409839892.26422.115.camel@edumazet-glaptop2.roam.corp.google.com>
On Do, 2014-09-04 at 07:11 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 15:40 +0200, Hannes Frederic Sowa wrote:
>
> > The type does not matter at all. Actually I wanted to use an empty
> > struct but was afraid it might not work on older compilers and didn't
> > want to check that with each version.
>
>
> It matters. Really.
>
> $ cat try.c
> #include <stdio.h>
>
> struct S_int {
> char a;
> int offset[0];
> char b:1;
> };
>
> struct S_char {
> char a;
> char offset[0];
> char b:1;
> };
>
> int main()
> {
> printf("sizeof(S_int) %zu\n", sizeof(struct S_int));
> printf("sizeof(S_char) %zu\n", sizeof(struct S_char));
> return 0;
> }
> $ gcc -o try try.c && ./try
> sizeof(S_int) 8
> sizeof(S_char) 2
Sure, compiler must make sure to correctly align structure for all
members (so it must round up). But this is not an issue with struct
sk_buff. I agree that switching to __u8 for offset marker seems to
reduce confusion.
Statically compiling skb_pkt_type_mask as I proposed above didn't work
out.
New patch, what do you think?
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..0fd3f95 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -765,27 +765,6 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
return (u64)err << 32 | ntohl(ret);
}
-#ifdef __BIG_ENDIAN_BITFIELD
-#define PKT_TYPE_MAX (7 << 5)
-#else
-#define PKT_TYPE_MAX 7
-#endif
-static int pkt_type_offset(void)
-{
- struct sk_buff skb_probe = {
- .pkt_type = ~0,
- };
- u8 *ct = (u8 *)&skb_probe;
- unsigned int off;
-
- for (off = 0; off < sizeof(struct sk_buff); off++) {
- if (ct[off] == PKT_TYPE_MAX)
- return off;
- }
- pr_err_once("Please fix pkt_type_offset(), as pkt_type couldn't be found\n");
- return -1;
-}
-
static int build_body(struct jit_ctx *ctx)
{
void *load_func[] = {jit_get_skb_b, jit_get_skb_h, jit_get_skb_w};
@@ -1332,11 +1311,7 @@ jmp_cmp:
case BPF_ANC | SKF_AD_PKTTYPE:
ctx->flags |= SEEN_SKB;
- off = pkt_type_offset();
-
- if (off < 0)
- return -1;
- emit_load_byte(r_tmp, r_skb, off, ctx);
+ emit_load_byte(r_tmp, r_skb, skb_pkt_type_offset(), ctx);
/* Keep only the last 3 bits */
emit_andi(r_A, r_tmp, PKT_TYPE_MAX, ctx);
#ifdef __BIG_ENDIAN_BITFIELD
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 61e45b7..8039bb8 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -223,37 +223,6 @@ static void bpf_jit_epilogue(struct bpf_jit *jit)
EMIT2(0x07fe);
}
-/* Helper to find the offset of pkt_type in sk_buff
- * Make sure its still a 3bit field starting at the MSBs within a byte.
- */
-#define PKT_TYPE_MAX 0xe0
-static int pkt_type_offset;
-
-static int __init bpf_pkt_type_offset_init(void)
-{
- struct sk_buff skb_probe = {
- .pkt_type = ~0,
- };
- char *ct = (char *)&skb_probe;
- int off;
-
- pkt_type_offset = -1;
- for (off = 0; off < sizeof(struct sk_buff); off++) {
- if (!ct[off])
- continue;
- if (ct[off] == PKT_TYPE_MAX)
- pkt_type_offset = off;
- else {
- /* Found non matching bit pattern, fix needed. */
- WARN_ON_ONCE(1);
- pkt_type_offset = -1;
- return -1;
- }
- }
- return 0;
-}
-device_initcall(bpf_pkt_type_offset_init);
-
/*
* make sure we dont leak kernel information to user
*/
@@ -753,12 +722,10 @@ call_fn: /* lg %r1,<d(function)>(%r13) */
}
break;
case BPF_ANC | SKF_AD_PKTTYPE:
- if (pkt_type_offset < 0)
- goto out;
/* lhi %r5,0 */
EMIT4(0xa7580000);
/* ic %r5,<d(pkt_type_offset)>(%r2) */
- EMIT4_DISP(0x43502000, pkt_type_offset);
+ EMIT4_DISP(0x43502000, skb_pkt_type_offset());
/* srl %r5,5 */
EMIT4_DISP(0x88500000, 5);
break;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..944e931 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -548,11 +548,25 @@ struct sk_buff {
ip_summed:2,
nohdr:1,
nfctinfo:3;
+
+ /* pkt_type bitfield is relatively addressed from bpf engines:
+ * if you reorder skb contents, ensure to update PKT_TYPE_MAX
+ * and keep __pkt_type_offset marker right in front of bitfield
+ * which does contain pkt_type member.
+ */
+
+#ifdef __BIG_ENDIAN_BITFIELD
+#define PKT_TYPE_MAX (7 << 5)
+#else
+#define PKT_TYPE_MAX 7
+#endif
+ __u8 __pkt_type_offset[0];
__u8 pkt_type:3,
fclone:2,
ipvs_property:1,
peeked:1,
nf_trace:1;
+
kmemcheck_bitfield_end(flags1);
__be16 protocol;
@@ -647,6 +661,11 @@ struct sk_buff {
#define SKB_ALLOC_FCLONE 0x01
#define SKB_ALLOC_RX 0x02
+static inline size_t skb_pkt_type_offset(void)
+{
+ return offsetof(struct sk_buff, __pkt_type_offset);
+}
+
/* Returns true if the skb was allocated from PFMEMALLOC reserves */
static inline bool skb_pfmemalloc(const struct sk_buff *skb)
{
diff --git a/net/core/filter.c b/net/core/filter.c
index d814b8a..ca64d7a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -87,30 +87,6 @@ int sk_filter(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(sk_filter);
-/* Helper to find the offset of pkt_type in sk_buff structure. We want
- * to make sure its still a 3bit field starting at a byte boundary;
- * taken from arch/x86/net/bpf_jit_comp.c.
- */
-#ifdef __BIG_ENDIAN_BITFIELD
-#define PKT_TYPE_MAX (7 << 5)
-#else
-#define PKT_TYPE_MAX 7
-#endif
-static unsigned int pkt_type_offset(void)
-{
- struct sk_buff skb_probe = { .pkt_type = ~0, };
- u8 *ct = (u8 *) &skb_probe;
- unsigned int off;
-
- for (off = 0; off < sizeof(struct sk_buff); off++) {
- if (ct[off] == PKT_TYPE_MAX)
- return off;
- }
-
- pr_err_once("Please fix %s, as pkt_type couldn't be found!\n", __func__);
- return -1;
-}
-
static u64 __skb_get_pay_offset(u64 ctx, u64 a, u64 x, u64 r4, u64 r5)
{
return __skb_get_poff((struct sk_buff *)(unsigned long) ctx);
@@ -191,7 +167,7 @@ static bool convert_bpf_extensions(struct sock_filter *fp,
case SKF_AD_OFF + SKF_AD_PKTTYPE:
*insn = BPF_LDX_MEM(BPF_B, BPF_REG_A, BPF_REG_CTX,
- pkt_type_offset());
+ skb_pkt_type_offset());
if (insn->off < 0)
return false;
insn++;
^ permalink raw reply related
* Re: [Patch net-next 08/11] net:fec: change FEC alignment to 64 bytes for ARM platform
From: Zhi Li @ 2014-09-04 14:17 UTC (permalink / raw)
To: David Laight
Cc: fugang.duan@freescale.com, Frank.Li@freescale.com,
davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174872BE@AcuExch.aculab.com>
On Thu, Sep 4, 2014 at 4:18 AM, David Laight <David.Laight@aculab.com> wrote:
> From: fugang.duan@freescale.com
>> >From: Frank Li
>> >> From: Fugang Duan <B38611@freescale.com>
>> >>
>> >> Since enet-avb has 64 bytes alignment limitation for rx DMA transfer.
>> >> The previous enet IP for ARM platform has 16 bytes alignment for tx
>> >> DMA transfer.
>> >
>> >Do you mean rx or tx here? or both??
>> >
>> >And can we beat up the hardware designers to stop these restrictions on rx
>> >(in particular) ethernet buffer alignments?
>> >A device isn't suitable for ethernet unless is can write the destination
>> >mac address to a 4n+2 boundary.
>> >
>> > David
>> >
>> Hi, David,
>>
>> For previous enet IP there has 16 bytes data buffer alignment limitation for tx & rx DMA transfer.
>> For imx6sx enet-avb IP, there has 64 bytes data buffer alignment limitation for rx DMA transfer, byte
>> alignment for tx data
>> Buffer for DMA transfer.
>>
>> I think rx data buffer alignment limitation don't introduce performance drop, is not complex for sw
>> implemention. Anyway,
>> We can request IC designer to remove the limitation for future chips.
>
> The 'problem' is that you need the IP header to be 32bit aligned.
> If the dma buffer has to be aligned then the code either has to do an expensive
> misaligned copy of the entire frame (at some point the entire frame is almost
> certainly all copied into an aligned buffer), or take the hit of misaligned
> memory accesses (which might have to be emulated with byte transfers).
>
> The rx buffer can be aligned provided the hardware skips (or writes junk to)
> the first two bytes.
ENET IP have feature to add additional 2 byte when save data to memory.
RX FIFO Shift-16
When this field is set, the actual frame data starts at bit 16 of the
first word read from the RX FIFO aligning
the Ethernet payload on a 32-bit boundary.
NOTE: This function only affects the FIFO storage and has no influence
on the statistics, which use the
actual length of the frame received.
0 Disabled.
1 Instructs the MAC to write two additional bytes in front of each
frame received into the RX FIFO.
i.MX6SX,
Remove TX alignment requirement.
But RX and DMA BD address is 64byte aligment.
best regards
Frank Li
>
> David
>
>
>
^ permalink raw reply
* [PATCH net-next] net-timestamp: fix allocation error in test
From: Willem de Bruijn @ 2014-09-04 14:15 UTC (permalink / raw)
To: netdev; +Cc: davem, dborkman, Willem de Bruijn
A buffer is incorrectly zeroed to the length of the pointer. If
cfg_payload_len < sizeof(void *) this can overwrites unrelated memory.
The buffer contents are never read, so no need to zero.
Fixes: 8fe2f761cae9 ("net-timestamp: expand documentation")
Reported-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
Documentation/networking/timestamping/txtimestamp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/networking/timestamping/txtimestamp.c b/Documentation/networking/timestamping/txtimestamp.c
index e5b0b98..b32fc2a 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c
@@ -192,7 +192,6 @@ static int recv_errmsg(int fd)
memset(&msg, 0, sizeof(msg));
memset(&entry, 0, sizeof(entry));
memset(ctrl, 0, sizeof(ctrl));
- memset(data, 0, sizeof(data));
entry.iov_base = data;
entry.iov_len = cfg_payload_len;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Eric Dumazet @ 2014-09-04 14:11 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <1409838010.23465.16.camel@localhost>
On Thu, 2014-09-04 at 15:40 +0200, Hannes Frederic Sowa wrote:
> The type does not matter at all. Actually I wanted to use an empty
> struct but was afraid it might not work on older compilers and didn't
> want to check that with each version.
It matters. Really.
$ cat try.c
#include <stdio.h>
struct S_int {
char a;
int offset[0];
char b:1;
};
struct S_char {
char a;
char offset[0];
char b:1;
};
int main()
{
printf("sizeof(S_int) %zu\n", sizeof(struct S_int));
printf("sizeof(S_char) %zu\n", sizeof(struct S_char));
return 0;
}
$ gcc -o try try.c && ./try
sizeof(S_int) 8
sizeof(S_char) 2
^ permalink raw reply
* Re: [PATCH] net: ethernet: cpsw: improve interrupt lookup logic in cpsw_probe()
From: Mugunthan V N @ 2014-09-04 14:06 UTC (permalink / raw)
To: Daniel Mack, davem; +Cc: julia.lawall, netdev, george.cherian, Daniel Mack
In-Reply-To: <1409814023-10189-1-git-send-email-daniel@zonque.org>
On Thursday 04 September 2014 12:30 PM, Daniel Mack wrote:
> From: Daniel Mack <zonque@gmail.com>
>
> Simplify the interrupt resource lookup code in cpsw_probe() by the
> following:
>
> * Only look at the first member of the resource. As the driver only
> works for DT-enabled platforms anyway, a resource of type
> IORESOURCE_IRQ will only contain one single entry
> (res->start == res->end), so there is no need for the iteration.
>
> * Add a bounds check to avoid overflows if we are passed more than
> ARRAY_SIZE(priv->irqs_table) resources.
>
> * Assign 'ret' with the return value of devm_request_irq() so that
> cpsw_probe() returns the appropriate error code.
>
> * If devm_request_irq() fails, report the error code in the log
> message.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Mugunthan V N <mugunthanvnm@gmail.com>
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH net-next] net-timestamp: expand documentation
From: Willem de Bruijn @ 2014-09-04 14:03 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Network Development, David Miller
In-Reply-To: <54086164.6030603@redhat.com>
>> +
>> + data = malloc(cfg_payload_len);
>> + if (!data)
>> + error(1, 0, "malloc");
>> +
>> + memset(&msg, 0, sizeof(msg));
>> + memset(&entry, 0, sizeof(entry));
>> + memset(ctrl, 0, sizeof(ctrl));
>> + memset(data, 0, sizeof(data));
>
>
> I think this memset is wrong and should rather be
> something like :
>
> memset(data, 0, cfg_payload_len);
>
Thanks, Daniel. I'll send a fix.
^ permalink raw reply
* Re: [Patch net-next 04/11] net:fec: add multiqueue support
From: Zhi Li @ 2014-09-04 14:02 UTC (permalink / raw)
To: Florian Fainelli
Cc: Frank Li, Duan Fugang-B38611, David Miller,
netdev@vger.kernel.org, Shawn Guo,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <CAGVrzcaB=7qkXzZ46-86-obMJBTOZQ01N7PROdi9RrkiDnE2zQ@mail.gmail.com>
On Wed, Sep 3, 2014 at 11:09 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2014-09-03 20:41 GMT-07:00 Zhi Li <lznuaa@gmail.com>:
>> On Wed, Sep 3, 2014 at 4:27 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 09/03/2014 01:38 PM, Frank Li wrote:
>>>> From: Fugang Duan <B38611@freescale.com>
>>>>
>>>> Add multiqueue support, which is compatible with previous single queue
>>>> driver for FEC/ENET IPs.
>>>>
>>>> By default, the tx/rx queue number is 1, user can config the queue number
>>>> at DTS file like this:
>>>> fsl,num_tx_queues=<3>;
>>>> fsl,num_rx_queues=<3>;
>>>>
>>>> Ethernet multiqueue mechanism can improve performance in SMP system.
>>>> For single hw queue, multiqueue can balance cpu loading.
>>>> For multi hw queues, multiple cores can process network packets in parallel,
>>>> and refer the article for the detail advantage for multiqueue:
>>>> http://vger.kernel.org/~davem/davem_nyc09.pdf
>>>
>>> You should probably split that patch in a minimum of two, or more
>>> different parts.
>>>
>>> Two parts:
>>>
>>> - add TX multiqueue support
>>> - add RX multiqueue support
>>>
>>
>> I understood your concern.
>> We also need consider logical integration.
>
> Sure, hence the reason why I suggest you make it happen in different
> parts, what you can do is make sure that you parse "fsl,num_tx_queues"
> and "fsl,num_rx_queues" as one of the last patches in this patch
> series. That way, we achieve a logical breakdown of the changes and
> only support and enable multiqueue when the driver is ready.
I can remove this part to "net:fec: add enet-avb IP support", last
patch just change DT file,
which come into difference maintainer.
But RX and TX is difficult to separate because RX and TX share some
function, such as
fec_enet_get_nextdesc
If I change TX part, fec_enet_get_nextdesc will add a parameter. If
that, all RX part need be
changed.
>
>>
>> Separate this patch just reduce changed lines in a patch.
>
> That is exactly my point, right now there is too much churn to make a
> reviewer able to spot potential problems.
>
>> Most of this patch change is from fep-> to txq-> or rxq->
>>
>> best regards
>> Frank Li
>>
>>> More parts:
>>>
>>> - prepare for TX multiqueue (introduce queue structure, definitions)
>>> - transition TX path to use queue 0
>>> - add support for the two other TX queues
>>> - prepare for RX multiqueue (introduce queue structure, definitions)
>>> - prepare for multiqueue, hardcode RX path to use queue 0
>>> - add support for the two other RX queues
>>>
>>> The number of changes are extremely hard to review...
>>> --
>>> Florian
>
>
>
> --
> Florian
^ permalink raw reply
* Re: [Patch net-next 10/11] ARM: Documentation: Update fec dts binding doc
From: Zhi Li @ 2014-09-04 13:47 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Frank.Li@freescale.com >> Frank Li, Duan Fugang-B38611,
David Miller, netdev@vger.kernel.org, Shawn Guo,
linux-arm-kernel@lists.infradead.org, devicetree
In-Reply-To: <54086337.2050602@cogentembedded.com>
On Thu, Sep 4, 2014 at 8:03 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hyphens
Okay, I will change it in next version.
^ permalink raw reply
* Re: [RFC net-next PATCH V2 0/3] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Jamal Hadi Salim @ 2014-09-04 13:44 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Alexander Duyck, John Fastabend
In-Reply-To: <20140904125247.4108.8132.stgit@dragon>
On 09/04/14 08:54, Jesper Dangaard Brouer wrote:
> Wanted people to review this work early... so this is the current
> state, even added my debug patch, if people want to "see" it work.
>
It would be useful to collect some stats when you are
experimenting with this stuff on the average batch size the driver
sees. Maybe a histogram which shows >1, > 3, > 10 etc that a
driver was able to chew on at a time. I realize this may skew
your results a little because you have to chew cycles updating
such a histogram.
I believe that any dumb traffic generator will show lots of packets
being batched. But that is not very interesting.
It would be hard to show more than 1 in the general case for
other types of traffic - but learning what else needs to be tweaked
to achive more batch sizes would be helpful.
i.e this would help quantify whether the batching is valuable in the
general sense...
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 13:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <1409836154.26422.101.camel@edumazet-glaptop2.roam.corp.google.com>
On Do, 2014-09-04 at 06:09 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 13:50 +0200, Hannes Frederic Sowa wrote:
>
> >
> > Denis, I thought about something like this, you can incorperate this in
> > your patch if you can give it a test and check for other architectures,
> > thanks!
> >
>
>
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 02529fc..87b86aa 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -548,6 +548,10 @@ struct sk_buff {
> > ip_summed:2,
> > nohdr:1,
> > nfctinfo:3;
> > + /* __pkt_type_offset marks the offset of the bitfield pkt_type
> > + * contains, so bpf can relatively address it
> > + */
> > + int __pkt_type_offset[0];
>
> Why is it 'int', while the following uses __u8 ?
The type does not matter at all. Actually I wanted to use an empty
struct but was afraid it might not work on older compilers and didn't
want to check that with each version.
> This means pkt_type has to stay at this bit offset, and nothing will
> detect at compile time or execution time if someone did a reorg or added
> a new field (it seems to be the trend lately)
>
> __u8 bar:4,
> pkt_type:3,
> ...
That would be fine, one just has to adapt the PKT_TYPE_MAX macros in
case on reorders, __pkt_type_offset only needs to stay just in front of
the bitfield pkt_type is located in. I think we should move them up into
struct sk_buff, so they belong together and people see that they need to
change those in case they reorder fields.
I still think about using anonymous unions, they might let us compute
the pkt_type mask at compiletime, I think.
Also, maybe we can compute offset at compiletime if we switch to
anonymous union.
static inline unsigned int skb_pkt_type_offset(void)
{
unsigned int mask;
struct sk_buff skb = {.pkt_type = ~0U};
mask = skb.__flags2;
BUILD_BUG_ON(!__builtin_constant_p(mask));
return mask;
}
something like that, haven't checked if that works.
>
>
>
> > __u8 pkt_type:3,
> > fclone:2,
> > ipvs_property:1,
> > @@ -647,6 +651,17 @@ struct sk_buff {
> > #define SKB_ALLOC_FCLONE 0x01
> > #define SKB_ALLOC_RX 0x02
> >
>
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > +#define PKT_TYPE_MAX (7 << 5)
> > +#else
> > +#define PKT_TYPE_MAX 7
> > +#endif
>
> It had a sense right before static int pkt_type_offset(void), but here,
> a casual reader might be lost.
Do you think we should just move PKT_TYPE_MAX macros right to the
definition of __pkt_type_offset?
> I am saying this because a reorder of the bit fields is probably needed
> to speedup __copy_skb_header() : Grouping together bit fields could
> allow optimized word copies instead of bit field manipulations.
Yes, I agree with you. It should be obvious to adapt the macros in case
someones reorders sk_buff. Either BUILD_BUG_ON or very obvious comments
+ declarations.
Thanks,
Hannes
^ permalink raw reply
* Re: [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Eric Dumazet @ 2014-09-04 13:29 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Tom Herbert, Hannes Frederic Sowa,
Florian Westphal, Daniel Borkmann, Jamal Hadi Salim,
Alexander Duyck, John Fastabend
In-Reply-To: <20140904125438.4108.38160.stgit@dragon>
On Thu, 2014-09-04 at 14:55 +0200, Jesper Dangaard Brouer wrote:
> Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
> sending/processing an entire skb list.
>
> This patch implements qdisc bulk dequeue, by allowing multiple packets
> to be dequeued in dequeue_skb().
>
> The optimization principle for this is two fold, (1) to amortize
> locking cost and (2) avoid expensive tailptr update for notifying HW.
> (1) Several packets are dequeued while holding the qdisc root_lock,
> amortizing locking cost over several packet. The dequeued SKB list is
> processed under the TXQ lock in dev_hard_start_xmit(), thus also
> amortizing the cost of the TXQ lock.
> (2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
> API to delay HW tailptr update, which also reduces the cost per
> packet.
>
> One restriction of the new API is that every SKB must belong to the
> same TXQ. This patch takes the easy way out, by restricting bulk
> dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
> qdisc only have attached a single TXQ.
>
> Some detail about the flow; dev_hard_start_xmit() will process the skb
> list, and transmit packets individually towards the driver (see
> xmit_one()). In case the driver stops midway in the list, the
> remaining skb list is returned by dev_hard_start_xmit(). In
> sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
>
> The patch also tries to limit the amount of bytes dequeued, based on
> the drivers BQL limits. It also tries to avoid and stop dequeuing
> when seeing a GSO packet (both real GSO and segmented GSO skb lists).
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> ---
> V2:
> - Restruct functions, split out functionality
> - Use BQL bytelimit to avoid overshooting driver limits, causing
> too large skb lists to be sitting on the requeue gso_skb.
>
> net/sched/sch_generic.c | 67 +++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 19696eb..a0c8070 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -56,6 +56,67 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
> return 0;
> }
>
> +static inline bool qdisc_may_bulk(const struct Qdisc *qdisc,
> + const struct sk_buff *skb)
Why skb is passed here ?
> +{
> + return (qdisc->flags & TCQ_F_ONETXQUEUE);
return qdisc->flags & TCQ_F_ONETXQUEUE;
> +}
> +
> +static inline struct sk_buff *qdisc_dequeue_validate(struct Qdisc *qdisc)
> +{
> + struct sk_buff *skb = qdisc->dequeue(qdisc);
> +
> + if (skb != NULL)
> + skb = validate_xmit_skb(skb, qdisc_dev(qdisc));
> +
> + return skb;
> +}
> +
> +static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
> + struct sk_buff *head)
> +{
> + struct sk_buff *new, *skb = head;
> + struct netdev_queue *txq = q->dev_queue;
> + int bytelimit = netdev_tx_avail_queue(txq);
> + int limit = 5;
> +
> + if (bytelimit <= 0)
> + return head;
> +
> + do {
> + if (skb->next || skb_is_gso(skb)) {
A GSO packet should not 'stop the processing' if the device is TSO
capable : It should look as a normal packet.
> + /* Stop processing if the skb is already a skb
> + * list (e.g a segmented GSO packet) or a real
> + * GSO packet */
> + break;
> + }
> + new = qdisc_dequeue_validate(q);
Thats broken.
After validate_xmit_skb() @new might be the head of a list. (GSO split)
> + if (new) {
> + skb->next = new;
> + skb = new;
and new is only the first element on the list.
> + bytelimit -= skb->len;
skb->len is the length of first segment.
> + cnt++;
> + /* One problem here is it is difficult to
> + * requeue the "new" dequeued skb, e.g. in
> + * case of GSO, thus a "normal" packet can
> + * have a GSO packet on its ->next ptr.
> + *
> + * Requeue is difficult because if requeuing
> + * on q->gso_skb, then a second requeue can
> + * happen from sch_direct_xmit e.g. if driver
> + * returns NETDEV_TX_BUSY, which would
> + * overwrite this requeue.
> + */
It should not overwrite, but insert back. Thats what need to be done.
> + }
> + } while (new && --limit && (bytelimit > 0));
> + skb = head;
> +
> + return skb;
> +}
> +
Idea is really the following :
Once qdisc dequeue bulk is done, and skb validated, we have a list of
skb to send to the device.
We iterate the list and try to send the individual skb.
As soon as device returns NETDEV_TX_BUSY (or even better, when the queue
is stopped), we requeue the remaining list.
BQL never tried to find the exact split point :
If available budget is 25000 bytes, and next TSO packet is 45000 bytes,
we send it.
So the bql validation should be done before the eventual
validate_xmit_skb() : This way you dont care of GSO or TSO.
^ permalink raw reply
* Re: [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Florian Westphal @ 2014-09-04 13:17 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Tom Herbert, Eric Dumazet,
Hannes Frederic Sowa, Florian Westphal, Daniel Borkmann,
Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125438.4108.38160.stgit@dragon>
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> Some detail about the flow; dev_hard_start_xmit() will process the skb
> list, and transmit packets individually towards the driver (see
> xmit_one()). In case the driver stops midway in the list, the
> remaining skb list is returned by dev_hard_start_xmit(). In
> sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
>
> The patch also tries to limit the amount of bytes dequeued, based on
> the drivers BQL limits. It also tries to avoid and stop dequeuing
> when seeing a GSO packet (both real GSO and segmented GSO skb lists).
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> +static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
> + struct sk_buff *head)
> +{
> + struct sk_buff *new, *skb = head;
> + struct netdev_queue *txq = q->dev_queue;
> + int bytelimit = netdev_tx_avail_queue(txq);
> + int limit = 5;
> +
> + if (bytelimit <= 0)
> + return head;
bytelimit < psched_mtu(qdisc_dev(q)) ?
It would avoid overshooting bql on next dequeue operation.
> + * Requeue is difficult because if requeuing
> + * on q->gso_skb, then a second requeue can
> + * happen from sch_direct_xmit e.g. if driver
> + * returns NETDEV_TX_BUSY, which would
> + * overwrite this requeue.
> + */
Perhaps we should just free the entire list in such a case;
it would also permit extending this to qdisc with
multiple tx queues later (since you could requeue skb
if the txq mapping changes)?
It would also allow use of qdisc_peek helpers to only dequeue
skb when it is suitable for bulking.
Unrelated to your patch: q->gso_skb should be renamed.
It has nothing to do with gso anymore...
^ permalink raw reply
* Re: [PATCH net-next 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-04 13:16 UTC (permalink / raw)
To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller
Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1409780851-32081-1-git-send-email-maheshb@google.com>
On 03/09/14 23:47, Mahesh Bandewar wrote:
> Earlier change to use usable slave array for TLB mode had an additional
> performance advantage. So extending the same logic to all other modes
> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
> Also consolidating this with the earlier TLB change.
>
> The main idea is to build the usable slaves array in the control path
> and use that array for slave selection during xmit operation.
>
> Measured performance in a setup with a bond of 4x1G NICs with 200
> instances of netperf for the modes involved (3ad, xor, tlb)
> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>
> Mode TPS-Before TPS-After
>
> 802.3ad : 468,694 493,101
> TLB (lb=0): 392,583 392,965
> XOR : 475,696 484,517
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
<<snip>>
Hi Mahesh,
I really like this idea, but I think this patch is far from ready and needs more
thorough examination.
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index f0f5eab0fab1..b0e8e7cfa10f 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
What happens to modes that don't have miimon enabled i.e. if I do: modprobe
bonding mode=2 and enslave something then the slave array will never get created
and I won't be able to xmit anything ever because miimon is 0 by default and
even if it wasn't I'll have to wait for it to create the slave array before I
can do any transmissions.
By the way, I just noticed that arp_interval is actually supported in XOR mode.
> @@ -1693,6 +1693,9 @@ static int __bond_release_one(struct net_device *bond_dev,
> if (BOND_MODE(bond) == BOND_MODE_8023AD)
> bond_3ad_unbind_slave(slave);
>
^^^^^^^^^^^^^^^^^^^^^^^
Empty line.
> + else if (BOND_MODE(bond) == BOND_MODE_XOR)
> + bond_update_slave_arr(bond, slave);
> +
> write_unlock_bh(&bond->lock);
>
> netdev_info(bond_dev, "Releasing %s interface %s\n",
> @@ -2009,6 +2012,9 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_UP);
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR)
> + bond_update_slave_arr(bond, NULL);
> +
> if (!bond->curr_active_slave ||
> (slave == bond->primary_slave))
> goto do_failover;
> @@ -2037,6 +2043,9 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_DOWN);
>
> + if (BOND_MODE(bond) == BOND_MODE_XOR)
> + bond_update_slave_arr(bond, slave);
> +
> if (slave == rcu_access_pointer(bond->curr_active_slave))
> goto do_failover;
>
> @@ -3149,6 +3158,7 @@ static int bond_open(struct net_device *bond_dev)
> static int bond_close(struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
> + struct bond_up_slave *arr;
>
> bond_work_cancel_all(bond);
> bond->send_peer_notif = 0;
> @@ -3156,6 +3166,10 @@ static int bond_close(struct net_device *bond_dev)
> bond_alb_deinitialize(bond);
> bond->recv_probe = NULL;
>
> + arr = rtnl_dereference(bond->slave_arr);
> + if (arr)
> + kfree_rcu(arr, rcu);
> +
^^^^^^^^^^^^^^
Bond close is dealt with, but what happens after: ip l set bond down, ip l set
bond up
or alternatively: ip l set bond down, echo -slave > slaves
The array is freed but slave_arr still points to it.
> return 0;
> }
>
> @@ -3684,15 +3698,84 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
> return NETDEV_TX_OK;
> }
>
> -/* In bond_xmit_xor() , we determine the output device by using a pre-
> - * determined xmit_hash_policy(), If the selected device is not enabled,
> - * find the next active slave.
> +/* Build the usable slaves array in control path for modes that use xmit-hash
> + * to determine the slave interface -
> + * (a) BOND_MODE_8023AD
> + * (b) BOND_MODE_XOR
> + * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
> */
> -static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
^^^^^^^^^^^^^^^^^^^^^^^^^^
In general any failing in this function means that the old array stays with the
old slave list. I think that you really should revisit the places that use this
after the patch and see if that won't cause any problems.
> {
> - struct bonding *bond = netdev_priv(bond_dev);
> + struct slave *slave;
> + struct list_head *iter;
> + struct bond_up_slave *new_arr, *old_arr;
> + int slaves_in_agg;
> + int agg_id = 0;
> + int ret = 0;
> +
> + new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
> + GFP_ATOMIC);
> + if (!new_arr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> + struct ad_info ad_info;
> +
> + if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Called with RCU otherwise sparse won't be happy. Perhaps
bond_3ad_get_active_agg_info() ?
> + pr_debug("__bond_3ad_get_active_agg_info failed\n");
> + kfree_rcu(new_arr, rcu);
> + ret = -EINVAL;
> + goto out;
> + }
> + slaves_in_agg = ad_info.ports;
> + agg_id = ad_info.aggregator_id;
> + }
> + bond_for_each_slave(bond, slave, iter) {
> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> + struct aggregator *agg;
> +
> + agg = SLAVE_AD_INFO(slave)->port.aggregator;
> + if (!agg || agg->aggregator_identifier != agg_id)
> + continue;
> + }
> + if (!bond_slave_can_tx(slave))
> + continue;
> + if (skipslave == slave)
> + continue;
> + new_arr->arr[new_arr->count++] = slave;
> + }
>
> - bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) % bond->slave_cnt);
> + old_arr = rcu_dereference_protected(bond->slave_arr,
> + lockdep_rtnl_is_held() ||
> + lockdep_is_held(&bond->lock) ||
> + lockdep_is_held(&bond->curr_slave_lock));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This line is the most troublesome for me, which lock is it ? Does this mean that
whichever I hold from the three I can update the slave array ?
I don't think this is worked out well, you should explicitly specify how and why
it is safe to update this under each of the locks and maybe you'll be able to
reduce the lock list :-)
> + rcu_assign_pointer(bond->slave_arr, new_arr);
> + if (old_arr)
> + kfree_rcu(old_arr, rcu);
> +
> +out:
> + return ret;
> +}
> +
> +/* Use this Xmit function for 3AD as well as XOR modes. The current
> + * usable slave array is formed in the control path. The xmit function
> + * just calculates hash and sends the packet out.
> + */
> +int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct bonding *bond = netdev_priv(dev);
> + struct slave *slave;
> + struct bond_up_slave *slaves;
> +
> + slaves = rcu_dereference(bond->slave_arr);
> + if (slaves && slaves->count) {
> + slave = slaves->arr[bond_xmit_hash(bond, skb) % slaves->count];
> + bond_dev_queue_xmit(bond, skb, slave->dev);
> + } else {
> + dev_kfree_skb_any(skb);
> + atomic_long_inc(&dev->tx_dropped);
> + }
>
> return NETDEV_TX_OK;
> }
> @@ -3794,12 +3877,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
> return bond_xmit_roundrobin(skb, dev);
> case BOND_MODE_ACTIVEBACKUP:
> return bond_xmit_activebackup(skb, dev);
> + case BOND_MODE_8023AD:
> case BOND_MODE_XOR:
> - return bond_xmit_xor(skb, dev);
> + return bond_3ad_xor_xmit(skb, dev);
> case BOND_MODE_BROADCAST:
> return bond_xmit_broadcast(skb, dev);
> - case BOND_MODE_8023AD:
> - return bond_3ad_xmit_xor(skb, dev);
> case BOND_MODE_ALB:
> return bond_alb_xmit(skb, dev);
> case BOND_MODE_TLB:
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index aace510d08d1..4a6195c0de60 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -177,6 +177,12 @@ struct slave {
> struct kobject kobj;
> };
>
> +struct bond_up_slave {
> + unsigned int count;
> + struct rcu_head rcu;
> + struct slave *arr[0];
> +};
> +
> /*
> * Link pseudo-state only used internally by monitors
> */
> @@ -196,6 +202,7 @@ struct bonding {
> struct slave __rcu *curr_active_slave;
> struct slave __rcu *current_arp_slave;
> struct slave *primary_slave;
> + struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
> bool force_primary;
> s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
> int (*recv_probe)(const struct sk_buff *, struct bonding *,
> @@ -527,6 +534,7 @@ const char *bond_slave_link_status(s8 link);
> struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> struct net_device *end_dev,
> int level);
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>
> #ifdef CONFIG_PROC_FS
> void bond_create_proc_entry(struct bonding *bond);
>
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Eric Dumazet @ 2014-09-04 13:09 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Denis Kirjanov, netdev@vger.kernel.org, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <1409831412.23465.3.camel@localhost>
On Thu, 2014-09-04 at 13:50 +0200, Hannes Frederic Sowa wrote:
>
> Denis, I thought about something like this, you can incorperate this in
> your patch if you can give it a test and check for other architectures,
> thanks!
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 02529fc..87b86aa 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -548,6 +548,10 @@ struct sk_buff {
> ip_summed:2,
> nohdr:1,
> nfctinfo:3;
> + /* __pkt_type_offset marks the offset of the bitfield pkt_type
> + * contains, so bpf can relatively address it
> + */
> + int __pkt_type_offset[0];
Why is it 'int', while the following uses __u8 ?
This means pkt_type has to stay at this bit offset, and nothing will
detect at compile time or execution time if someone did a reorg or added
a new field (it seems to be the trend lately)
__u8 bar:4,
pkt_type:3,
...
> __u8 pkt_type:3,
> fclone:2,
> ipvs_property:1,
> @@ -647,6 +651,17 @@ struct sk_buff {
> #define SKB_ALLOC_FCLONE 0x01
> #define SKB_ALLOC_RX 0x02
>
> +#ifdef __BIG_ENDIAN_BITFIELD
> +#define PKT_TYPE_MAX (7 << 5)
> +#else
> +#define PKT_TYPE_MAX 7
> +#endif
It had a sense right before static int pkt_type_offset(void), but here,
a casual reader might be lost.
I am saying this because a reorder of the bit fields is probably needed
to speedup __copy_skb_header() : Grouping together bit fields could
allow optimized word copies instead of bit field manipulations.
^ permalink raw reply
* Re: [Patch net-next 10/11] ARM: Documentation: Update fec dts binding doc
From: Sergei Shtylyov @ 2014-09-04 13:03 UTC (permalink / raw)
To: Frank.Li@freescale.com >> Frank Li, b38611, davem, netdev,
lznuaa
Cc: shawn.guo, linux-arm-kernel, devicetree, Frank Li
In-Reply-To: <54078488.68c1420a.3020.7c0dSMTPIN_ADDED_BROKEN@mx.google.com>
Hello.
[Resending since the previous message bounced from the FSL address...]
On 9/4/2014 12:34 AM, y@shlinux1.ap.freescale.net wrote:
> From: Frank Li <Frank.Li@freescale.com>
> This patch update fec devicetree binding doc that add Optional
> properties "fsl,num_tx_queues" and "fsl,num_rx_queues".
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
> Documentation/devicetree/bindings/net/fsl-fec.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index 8a2c7b5..4da148d 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -16,6 +16,12 @@ Optional properties:
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> Use instead of phy-handle.
> +- fsl,num_tx_queues : The property is valid for enet-avb IP, which supports
> + hw multi queues. Should specify the tx queue number, otherwise set tx queue
> + number to 1.
> +- fsl,num_rx_queues : The property is valid for enet-avb IP, which supports
> + hw multi queues. Should specify the rx queue number, otherwise set rx queue
> + number to 1.
Hyphens are preferred to underscores in the prop names.
WBR, Sergei
^ permalink raw reply
* [RFC net-next PATCH V2 3/3] qdisc: debug statements while testing prev-patch
From: Jesper Dangaard Brouer @ 2014-09-04 12:56 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125247.4108.8132.stgit@dragon>
Not-signed-off
---
net/sched/sch_generic.c | 28 ++++++++++++++++++++++++++++
1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a0c8070..8c8ac40 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -47,8 +47,20 @@ EXPORT_SYMBOL(default_qdisc_ops);
static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
{
+ int bytelimit = netdev_tx_avail_queue(q->dev_queue); //DEBUG
+
skb_dst_force(skb);
q->gso_skb = skb;
+
+ if (skb->next) // DEBUG
+ net_warn_ratelimited(
+ "%s() dev:%s REQUEUEd SKB list len:%d bql:%d\n",
+ __func__, q->dev_queue->dev->name, bytelimit, skb->len);
+ else if (skb_is_gso(skb)) // DEBUG
+ net_warn_ratelimited(
+ "%s() dev:%s REQUEUEd GSO len:%d bql:%d\n",
+ __func__, q->dev_queue->dev->name, bytelimit, skb->len);
+
q->qstats.requeues++;
q->q.qlen++; /* it's still part of the queue */
__netif_schedule(q);
@@ -76,9 +88,11 @@ static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
struct sk_buff *head)
{
struct sk_buff *new, *skb = head;
+//?? struct netdev_queue *txq = skb_get_tx_queue(dev, skb); //which to choose?
struct netdev_queue *txq = q->dev_queue;
int bytelimit = netdev_tx_avail_queue(txq);
int limit = 5;
+ int cnt = 0; //DEBUG
if (bytelimit <= 0)
return head;
@@ -107,10 +121,24 @@ static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
* returns NETDEV_TX_BUSY, which would
* overwrite this requeue.
*/
+ if (skb->next) //DEBUG
+ net_warn_ratelimited(
+ "%s() dev:%s pkt-append SKB-list bql:%d cnd:%d\n",
+ __func__, q->dev_queue->dev->name,
+ bytelimit, cnt);
+ else if (skb_is_gso(skb))
+ net_warn_ratelimited(
+ "%s() dev:%s pkt-append real-GSO bql:%d cnd:%d\n",
+ __func__, q->dev_queue->dev->name,
+ bytelimit, cnt);
}
} while (new && --limit && (bytelimit > 0));
skb = head;
+ if (cnt > 0) //DEBUG
+ net_warn_ratelimited("%s() dev:%s BULK-active deq:%d bql:%d\n",
+ __func__, q->dev_queue->dev->name,
+ cnt, bytelimit);
return skb;
}
^ permalink raw reply related
* Re: [PATCH net-next] net-timestamp: expand documentation
From: Daniel Borkmann @ 2014-09-04 12:56 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem
In-Reply-To: <1409534867-32710-1-git-send-email-willemb@google.com>
Hi Willem,
On 09/01/2014 03:27 AM, Willem de Bruijn wrote:
> Expand Documentation/networking/timestamping.txt with new
> interfaces and bytestream timestamping. Also minor
> cleanup of the other text.
>
> Import txtimestamp.c test of the new features.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
...
> +static int recv_errmsg(int fd)
> +{
> + static char ctrl[1024 /* overprovision*/];
> + static struct msghdr msg;
> + struct iovec entry;
> + static char *data;
> + int ret = 0;
> +
> + data = malloc(cfg_payload_len);
> + if (!data)
> + error(1, 0, "malloc");
> +
> + memset(&msg, 0, sizeof(msg));
> + memset(&entry, 0, sizeof(entry));
> + memset(ctrl, 0, sizeof(ctrl));
> + memset(data, 0, sizeof(data));
I think this memset is wrong and should rather be
something like :
memset(data, 0, cfg_payload_len);
> + entry.iov_base = data;
> + entry.iov_len = cfg_payload_len;
> + msg.msg_iov = &entry;
> + msg.msg_iovlen = 1;
> + msg.msg_name = NULL;
> + msg.msg_namelen = 0;
> + msg.msg_control = ctrl;
> + msg.msg_controllen = sizeof(ctrl);
^ permalink raw reply
* [RFC net-next PATCH V2 2/3] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jesper Dangaard Brouer @ 2014-09-04 12:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140904125247.4108.8132.stgit@dragon>
Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
sending/processing an entire skb list.
This patch implements qdisc bulk dequeue, by allowing multiple packets
to be dequeued in dequeue_skb().
The optimization principle for this is two fold, (1) to amortize
locking cost and (2) avoid expensive tailptr update for notifying HW.
(1) Several packets are dequeued while holding the qdisc root_lock,
amortizing locking cost over several packet. The dequeued SKB list is
processed under the TXQ lock in dev_hard_start_xmit(), thus also
amortizing the cost of the TXQ lock.
(2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
API to delay HW tailptr update, which also reduces the cost per
packet.
One restriction of the new API is that every SKB must belong to the
same TXQ. This patch takes the easy way out, by restricting bulk
dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
qdisc only have attached a single TXQ.
Some detail about the flow; dev_hard_start_xmit() will process the skb
list, and transmit packets individually towards the driver (see
xmit_one()). In case the driver stops midway in the list, the
remaining skb list is returned by dev_hard_start_xmit(). In
sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
The patch also tries to limit the amount of bytes dequeued, based on
the drivers BQL limits. It also tries to avoid and stop dequeuing
when seeing a GSO packet (both real GSO and segmented GSO skb lists).
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
V2:
- Restruct functions, split out functionality
- Use BQL bytelimit to avoid overshooting driver limits, causing
too large skb lists to be sitting on the requeue gso_skb.
net/sched/sch_generic.c | 67 +++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 19696eb..a0c8070 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,67 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
return 0;
}
+static inline bool qdisc_may_bulk(const struct Qdisc *qdisc,
+ const struct sk_buff *skb)
+{
+ return (qdisc->flags & TCQ_F_ONETXQUEUE);
+}
+
+static inline struct sk_buff *qdisc_dequeue_validate(struct Qdisc *qdisc)
+{
+ struct sk_buff *skb = qdisc->dequeue(qdisc);
+
+ if (skb != NULL)
+ skb = validate_xmit_skb(skb, qdisc_dev(qdisc));
+
+ return skb;
+}
+
+static inline struct sk_buff *qdisc_bulk_dequeue_skb(struct Qdisc *q,
+ struct sk_buff *head)
+{
+ struct sk_buff *new, *skb = head;
+ struct netdev_queue *txq = q->dev_queue;
+ int bytelimit = netdev_tx_avail_queue(txq);
+ int limit = 5;
+
+ if (bytelimit <= 0)
+ return head;
+
+ do {
+ if (skb->next || skb_is_gso(skb)) {
+ /* Stop processing if the skb is already a skb
+ * list (e.g a segmented GSO packet) or a real
+ * GSO packet */
+ break;
+ }
+ new = qdisc_dequeue_validate(q);
+ if (new) {
+ skb->next = new;
+ skb = new;
+ bytelimit -= skb->len;
+ cnt++;
+ /* One problem here is it is difficult to
+ * requeue the "new" dequeued skb, e.g. in
+ * case of GSO, thus a "normal" packet can
+ * have a GSO packet on its ->next ptr.
+ *
+ * Requeue is difficult because if requeuing
+ * on q->gso_skb, then a second requeue can
+ * happen from sch_direct_xmit e.g. if driver
+ * returns NETDEV_TX_BUSY, which would
+ * overwrite this requeue.
+ */
+ }
+ } while (new && --limit && (bytelimit > 0));
+ skb = head;
+
+ return skb;
+}
+
+/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
+ * A requeued skb (via q->gso_skb) can also be a SKB list.
+ */
static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
{
struct sk_buff *skb = q->gso_skb;
@@ -71,9 +132,9 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
skb = NULL;
} else {
if (!(q->flags & TCQ_F_ONETXQUEUE) || !netif_xmit_frozen_or_stopped(txq)) {
- skb = q->dequeue(q);
- if (skb)
- skb = validate_xmit_skb(skb, qdisc_dev(q));
+ skb = qdisc_dequeue_validate(q);
+ if (skb && qdisc_may_bulk(q, skb))
+ skb = qdisc_bulk_dequeue_skb(q, skb);
}
}
^ 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