Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Marcel Holtmann @ 2015-02-04 21:09 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David Laight, Chen Gang S, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <54D27D68.7040501-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

Hi Sergei,

>>> -static inline int hci_test_bit(int nr, void *addr)
>>> +static inline int hci_test_bit(int nr, const void *addr)
>>>  {
>>>  	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>>>  }
> 
>> Is there a 'standard' function lurking that will do the above.
>> On x86 the cpus 'bit test' instruction will handle bit numbers
>> greater than the word size - so it can be a single instruction.
> 
>   Of course, there's test_bit().

we did leave hci_test_bit in the code since there are some userspace facing API that we can not change. Remember that the origin of this code is from 2.4.6 kernel.

So we can only change this if you can ensure not to break the userspace API. So might want to write unit tests to ensure working HCI filter before even considering touching this.

Regards

Marcel

^ permalink raw reply

* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-04 21:11 UTC (permalink / raw)
  To: Michal Kazior
  Cc: Neal Cardwell, linux-wireless, Network Development,
	eyalpe-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb
In-Reply-To: <1423056591.907.130.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>

I do not see how a TSO patch could hurt a flow not using TSO/GSO.

This makes no sense.

ath10k tx completions being batched/deferred to a tasklet might increase
probability to hit this condition in tcp_wfree() :

        /* If this softirq is serviced by ksoftirqd, we are likely under stress.
         * Wait until our queues (qdisc + devices) are drained.
         * This gives :
         * - less callbacks to tcp_write_xmit(), reducing stress (batches)
         * - chance for incoming ACK (processed by another cpu maybe)
         *   to migrate this flow (skb->ooo_okay will be eventually set)
         */
        if (wmem >= SKB_TRUESIZE(1) && this_cpu_ksoftirqd() == current)
                goto out;

Meaning tcp stack waits all skbs left qdisc/NIC queues before queuing
additional packets.

I would try to call skb_orphan() in ath10k if you really want to keep
these batches.

I have hard time to understand why tx completed packets go through
ath10k_htc_rx_completion_handler().. anyway...

Most conservative patch would be :

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9c782a42665e1aaf43bfbca441631ee58da50c09..6a36317d6bb0447202dee15528130bd5e21248c4 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1642,6 +1642,7 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_TX_COMPL_IND:
+		skb_orphan(skb);
 		spin_lock_bh(&htt->tx_lock);
 		__skb_queue_tail(&htt->tx_compl_q, skb);
 		spin_unlock_bh(&htt->tx_lock);


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] net: fs_enet: Implement NETIF_F_SG feature
From: David Miller @ 2015-02-04 21:13 UTC (permalink / raw)
  To: christophe.leroy
  Cc: pantelis.antoniou, vbordug, linux-kernel, linuxppc-dev, netdev
In-Reply-To: <20150202170654.D5EE21A5E90@localhost.localdomain>

From: Christophe Leroy <christophe.leroy@c-s.fr>
Date: Mon,  2 Feb 2015 18:06:54 +0100 (CET)

> Freescale ethernet controllers have the capability to re-assemble fragmented
> data into a single ethernet frame. This patch uses this capability and
> implements NETIP_F_SG feature into the fs_enet ethernet driver.
> 
> On a MPC885, I get 53% performance improvement on a ftp transfer of a 15Mb file:
>   * Without the patch : 2,8 Mbps
>   * With the patch : 4,3 Mbps
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied, thanks.

^ permalink raw reply

* [PATCH net] flow_keys: n_proto type should be __be16
From: Eric Dumazet @ 2015-02-04 21:31 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Govindarajulu Varadarajan

From: Eric Dumazet <edumazet@google.com>

(struct flow_keys)->n_proto is in network order, use
proper type for this.

Fixes following sparse errors :

net/core/flow_dissector.c:139:39: warning: incorrect type in assignment (different base types)
net/core/flow_dissector.c:139:39:    expected unsigned short [unsigned] [usertype] n_proto
net/core/flow_dissector.c:139:39:    got restricted __be16 [assigned] [usertype] proto
net/core/flow_dissector.c:237:23: warning: incorrect type in assignment (different base types)
net/core/flow_dissector.c:237:23:    expected unsigned short [unsigned] [usertype] n_proto
net/core/flow_dissector.c:237:23:    got restricted __be16 [assigned] [usertype] proto

Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: e0f31d849867 ("flow_keys: Record IP layer protocol in skb_flow_dissect()")
---
 include/net/flow_keys.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/flow_keys.h b/include/net/flow_keys.h
index 7ee2df083542..dc8fd81412bf 100644
--- a/include/net/flow_keys.h
+++ b/include/net/flow_keys.h
@@ -22,9 +22,9 @@ struct flow_keys {
 		__be32 ports;
 		__be16 port16[2];
 	};
-	u16 thoff;
-	u16 n_proto;
-	u8 ip_proto;
+	u16	thoff;
+	__be16	n_proto;
+	u8	ip_proto;
 };
 
 bool __skb_flow_dissect(const struct sk_buff *skb, struct flow_keys *flow,

^ permalink raw reply related

* [PATCH net] net: remove some sparse warnings
From: Eric Dumazet @ 2015-02-04 21:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

netdev_adjacent_add_links() and netdev_adjacent_del_links()
are static.

queue->qdisc has __rcu annotation, need to use RCU_INIT_POINTER()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index c87a2264a02b..7fe82929f509 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5294,7 +5294,7 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
-void netdev_adjacent_add_links(struct net_device *dev)
+static void netdev_adjacent_add_links(struct net_device *dev)
 {
 	struct netdev_adjacent *iter;
 
@@ -5319,7 +5319,7 @@ void netdev_adjacent_add_links(struct net_device *dev)
 	}
 }
 
-void netdev_adjacent_del_links(struct net_device *dev)
+static void netdev_adjacent_del_links(struct net_device *dev)
 {
 	struct netdev_adjacent *iter;
 
@@ -6627,7 +6627,7 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
 	if (!queue)
 		return NULL;
 	netdev_init_one_queue(dev, queue, NULL);
-	queue->qdisc = &noop_qdisc;
+	RCU_INIT_POINTER(queue->qdisc, &noop_qdisc);
 	queue->qdisc_sleeping = &noop_qdisc;
 	rcu_assign_pointer(dev->ingress_queue, queue);
 #endif

^ permalink raw reply related

* Re: [PATCH v2] net: bluetooth: hci_sock: Use 'const void *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-04 21:47 UTC (permalink / raw)
  To: Marcel Holtmann, Sergei Shtylyov
  Cc: David Laight, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
	linux-bluetooth@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <0AB99BEA-C379-439C-AD80-5F2122AE37E5@holtmann.org>

On 2/5/15 05:09, Marcel Holtmann wrote:
> Hi Sergei,
> 
>>>> -static inline int hci_test_bit(int nr, void *addr)
>>>> +static inline int hci_test_bit(int nr, const void *addr)
>>>>  {
>>>>  	return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31));
>>>>  }
>>
>>> Is there a 'standard' function lurking that will do the above.
>>> On x86 the cpus 'bit test' instruction will handle bit numbers
>>> greater than the word size - so it can be a single instruction.
>>
>>   Of course, there's test_bit().
> 
> we did leave hci_test_bit in the code since there are some userspace facing API that we can not change. Remember that the origin of this code is from 2.4.6 kernel.
> 
> So we can only change this if you can ensure not to break the userspace API. So might want to write unit tests to ensure working HCI filter before even considering touching this.
> 

For me, we have to remain hci_test_bit(), it is for "__u32 *" (which we
can not change). The common test_bit() is for "unsigned long *", in this
case, I guess it may cause issue under 64-bit environments.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

^ permalink raw reply

* Re: [PATCH net-next] bridge: Let bridge not age 'externally' learnt FDB entries, they are removed when 'external' entity notifies the aging
From: David Miller @ 2015-02-04 21:51 UTC (permalink / raw)
  To: siva.mannem.lnx; +Cc: netdev
In-Reply-To: <1422897714-5956-1-git-send-email-siva.mannem.lnx@gmail.com>

From: Siva Mannem <siva.mannem.lnx@gmail.com>
Date: Mon,  2 Feb 2015 22:51:54 +0530

>  When 'learned_sync' flag is turned on, the offloaded switch
>  port syncs learned MAC addresses to bridge's FDB via switchdev notifier
>  (NETDEV_SWITCH_FDB_ADD). Currently, FDB entries learnt via this mechanism are
>  wrongly being deleted by bridge aging logic. This patch ensures that FDB
>  entries synced from offloaded switch ports are not deleted by bridging logic.
>  Such entries can only be deleted via switchdev notifier
>  (NETDEV_SWITCH_FDB_DEL).
> 
> Signed-off-by: Siva Mannem <siva.mannem.lnx@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net: usb: sr9700: Use 'SR_' prefix for the common register macros
From: David Miller @ 2015-02-04 21:53 UTC (permalink / raw)
  To: gang.chen; +Cc: linux-usb, netdev, linux-kernel
In-Reply-To: <54CFE578.7030207@sunrus.com.cn>

From: Chen Gang S <gang.chen@sunrus.com.cn>
Date: Tue, 03 Feb 2015 05:00:40 +0800

> The commone register macors (e.g. RSR) is too commont to drivers, it may
> be conflict with the architectures (e.g. xtensa, sh).
> 
> The related warnings (with allmodconfig under xtensa):
> 
>     CC [M]  drivers/net/usb/sr9700.o
>   In file included from drivers/net/usb/sr9700.c:24:0:
>   drivers/net/usb/sr9700.h:65:0: warning: "RSR" redefined
>    #define RSR   0x06
>    ^
>   In file included from ./arch/xtensa/include/asm/bitops.h:22:0,
>                    from include/linux/bitops.h:36,
>                    from include/linux/kernel.h:10,
>                    from include/linux/list.h:8,
>                    from include/linux/module.h:9,
>                    from drivers/net/usb/sr9700.c:13:
>   ./arch/xtensa/include/asm/processor.h:190:0: note: this is the location of the previous definition
>    #define RSR(v,sr) __asm__ __volatile__ ("rsr %0,"__stringify(sr) : "=a"(v));
>    ^
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] net: add skb functions to process remote checksum offload
From: David Miller @ 2015-02-04 21:54 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <1422922054-22996-1-git-send-email-therbert@google.com>

From: Tom Herbert <therbert@google.com>
Date: Mon,  2 Feb 2015 16:07:34 -0800

> This patch adds skb_remcsum_process and skb_gro_remcsum_process to
> perform the appropriate adjustments to the skb when receiving
> remote checksum offload.
> 
> Updated vxlan and gue to use these functions.
> 
> Tested: Ran TCP_RR and TCP_STREAM netperf for VXLAN and GUE, did
> not see any change in performance.
> 
> Signed-off-by: Tom Herbert <therbert@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH v3] gianfar: correct the bad expression while writing bit-pattern
From: David Miller @ 2015-02-04 21:55 UTC (permalink / raw)
  To: sanjeev_sharma; +Cc: claudiu.manoil, matei.pavaluca, netdev, linux-kernel
In-Reply-To: <1422948722-16864-1-git-send-email-sanjeev_sharma@mentor.com>

From: Sanjeev Sharma <sanjeev_sharma@mentor.com>
Date: Tue, 3 Feb 2015 13:02:02 +0530

> This patch correct the bad expression while writing the
> bit-pattern from software's buffer to hardware registers.
> 
> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
> ---
> Changes in v3:
>   - corrected the expression by introducing tab->fe[i].prop.

Applied, thanks.

^ permalink raw reply

* Re: pull request: bluetooth-next 2015-02-03
From: David Miller @ 2015-02-04 21:56 UTC (permalink / raw)
  To: johan.hedberg; +Cc: netdev, linux-bluetooth
In-Reply-To: <20150203082123.GA13130@t440s.lan>

From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Tue, 3 Feb 2015 10:21:23 +0200

> Here's what's likely the last bluetooth-next pull request for 3.20.
> Notable changes include:
> 
>  - xHCI workaround + a new id for the ath3k driver
>  - Several new ids for the btusb driver
>  - Support for new Intel Bluetooth controllers
>  - Minor cleanups to ieee802154 code
>  - Nested sleep warning fix in socket accept() code path
>  - Fixes for Out of Band pairing handling
>  - Support for LE scan restarting for HCI_QUIRK_STRICT_DUPLICATE_FILTER
>  - Improvements to data we expose through debugfs
>  - Proper handling of Hardware Error HCI events
> 
> Please let me know if there are any issues pulling. Thanks.

Pulled, thanks Johan.

^ permalink raw reply

* Re: [PATCH 1/2] tlan: use msecs_to_jiffies for conversion
From: David Miller @ 2015-02-04 21:58 UTC (permalink / raw)
  To: hofrat; +Cc: chessman, netdev, linux-kernel
In-Reply-To: <1422953085-19797-1-git-send-email-hofrat@osadl.org>

From: Nicholas Mc Guire <hofrat@osadl.org>
Date: Tue,  3 Feb 2015 03:44:45 -0500

> This is only an API consolidation and should make things more readable
> it replaces var * HZ / 1000 by msecs_to_jiffies(var).
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] tlan: msecs_to_jiffies convrsion
From: David Miller @ 2015-02-04 21:58 UTC (permalink / raw)
  To: hofrat; +Cc: chessman, netdev, linux-kernel
In-Reply-To: <1422953111-19840-1-git-send-email-hofrat@osadl.org>

From: Nicholas Mc Guire <hofrat@osadl.org>
Date: Tue,  3 Feb 2015 03:45:11 -0500

> This is only an API consolidation and should make things more readable
> it replaces var * HZ / 1000 by msecs_to_jiffies(var).
> 
> As there is a discrepancy between the code and the comments this is in
> a separate patch.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>

Applied.

^ permalink raw reply

* Re: [PATCH for-3.19] vhost/net: fix up num_buffers endian-ness
From: David Miller @ 2015-02-04 21:59 UTC (permalink / raw)
  To: mst; +Cc: kvm, netdev, linux-kernel, virtualization, viro
In-Reply-To: <1422896842-19186-1-git-send-email-mst@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 3 Feb 2015 11:07:06 +0200

> In virtio 1.0 mode, when mergeable buffers are enabled on a big-endian
> host, num_buffers wasn't byte-swapped correctly, so large incoming
> packets got corrupted.
> 
> To fix, fill it in within hdr - this also makes sure it gets
> the correct type.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH] csiostor:Use firmware version from cxgb4/t4fw_version.h
From: David Miller @ 2015-02-04 22:00 UTC (permalink / raw)
  To: praveenm; +Cc: netdev, linux-scsi, JBottomley, hch, hariprasad
In-Reply-To: <1422964106-4033-1-git-send-email-praveenm@chelsio.com>

From: Praveen Madhavan <praveenm@chelsio.com>
Date: Tue,  3 Feb 2015 17:18:26 +0530

> This patch is to use firmware version macros from t4fw_version.h
> and also enables 40g T5 adapter.
> 
> Signed-off-by: Praveen Madhavan <praveenm@chelsio.com>

Applied.

^ permalink raw reply

* [PATCH net] pktgen: fix UDP checksum computation
From: Sabrina Dubroca @ 2015-02-04 22:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, tgraf, Sabrina Dubroca

This patch fixes two issues in UDP checksum computation in pktgen.

First, the pseudo-header uses the source and destination IP
addresses. Currently, the ports are used for IPv4.

Second, the UDP checksum covers both header and data.  So we need to
generate the data earlier (move pktgen_finalize_skb up), and compute
the checksum for UDP header + data.

Fixes: c26bf4a51308c ("pktgen: Add UDPCSUM flag to support UDP checksums")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/core/pktgen.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index da934fc3faa8..9fa25b0ea145 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2842,25 +2842,25 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
 	skb->dev = odev;
 	skb->pkt_type = PACKET_HOST;
 
+	pktgen_finalize_skb(pkt_dev, skb, datalen);
+
 	if (!(pkt_dev->flags & F_UDPCSUM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else if (odev->features & NETIF_F_V4_CSUM) {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum = 0;
-		udp4_hwcsum(skb, udph->source, udph->dest);
+		udp4_hwcsum(skb, iph->saddr, iph->daddr);
 	} else {
-		__wsum csum = udp_csum(skb);
+		__wsum csum = skb_checksum(skb, skb_transport_offset(skb), datalen + 8, 0);
 
 		/* add protocol-dependent pseudo-header */
-		udph->check = csum_tcpudp_magic(udph->source, udph->dest,
+		udph->check = csum_tcpudp_magic(iph->saddr, iph->daddr,
 						datalen + 8, IPPROTO_UDP, csum);
 
 		if (udph->check == 0)
 			udph->check = CSUM_MANGLED_0;
 	}
 
-	pktgen_finalize_skb(pkt_dev, skb, datalen);
-
 #ifdef CONFIG_XFRM
 	if (!process_ipsec(pkt_dev, skb, protocol))
 		return NULL;
@@ -2976,6 +2976,8 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 	skb->dev = odev;
 	skb->pkt_type = PACKET_HOST;
 
+	pktgen_finalize_skb(pkt_dev, skb, datalen);
+
 	if (!(pkt_dev->flags & F_UDPCSUM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else if (odev->features & NETIF_F_V6_CSUM) {
@@ -2984,7 +2986,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 		skb->csum_offset = offsetof(struct udphdr, check);
 		udph->check = ~csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, 0);
 	} else {
-		__wsum csum = udp_csum(skb);
+		__wsum csum = skb_checksum(skb, skb_transport_offset(skb), udplen, 0);
 
 		/* add protocol-dependent pseudo-header */
 		udph->check = csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, csum);
@@ -2993,8 +2995,6 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 			udph->check = CSUM_MANGLED_0;
 	}
 
-	pktgen_finalize_skb(pkt_dev, skb, datalen);
-
 	return skb;
 }
 
-- 
2.2.2

^ permalink raw reply related

* RE: [PATCH net] hyperv: Fix the error processing in netvsc_send()
From: Haiyang Zhang @ 2015-02-04 22:26 UTC (permalink / raw)
  To: Jason Wang
  Cc: olaf@aepfle.de, netdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1423034952.10558.3@smtp.corp.redhat.com>



> -----Original Message-----
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Wednesday, February 4, 2015 2:29 AM
> > The EAGAIN error doesn't normally happen, because we set the hi water
> > mark
> > to stop send queue.
> 
> This is not true since only txq was stopped which means only network
> stack stop sending packets but not for control path e.g
> rndis_filter_send_request() or other callers who call
> vmbus_sendpacket() directly (e.g recv completion).
> 
> For control path, user may meet several errors when they want to change
> mac address under heavy load.
> 
> What's more serious is netvsc_send_recv_completion(), it can not even
> recover from more than 3 times of EAGAIN.
> 
> I must say mixing data packets with control packets with the same
> channel sounds really scary. Since control packets could be blocked or
> even dropped because of data packets already queued during heavy load,
> and you need to synchronize two paths carefully (e.g I didn't see any
> tx lock were held if rndis_filter_send_request() call netsc_send()
> which may stop or start a queue).

The RING_AVAIL_PERCENT_HIWATER is defined to be 20, so the data traffic
can only occupy 20% of the ring buffer before stopping the txq. So, this
mechanism ensures the control messages are not blocked by data traffic.

> >  If in really rare case, the ring buffer is full and there
> > is no outstanding sends, we can't stop queue here because there will
> > be no
> > send-completion msg to wake it up.
> 
> Confused, I believe only txq is stopped but we may still get completion
> interrupt in this case.

If there is no outstanding sends in this queue (queue_sends[q_idx]), we 
won't receive any more send-completion msg.

> 
> > And, the ring buffer is likely to be
> > occupied by other special msg, e.g. receive-completion msg (not a
> > normal case),
> > so we can't assume there are available slots.
> 
> Then why not checking hv_ringbuf_avail_percent() instead? And there's
> no need to check queue_sends since it does not count recv completion.

When ret == -EAGAIN, which means the ring is full, we don't need to check
hv_ringbuf_avail_percent().

> > We don't request retry from
> > the upper layer in this case to avoid possible busy retry.
> 
> Can't we just do this by stopping txq and depending on tx interrupt to
> wake it?

There is no tx interrupt. Do you mean rx interrupt for the send-completion?

In usual cases, when we hit the high water mark, the stopped queue depends on
the send-completion msg to wake up. But, not in some special cases.
As said above, we won't receive any more send-completion msg when there is 
no outstanding sends in this queue.

Thanks,
- Haiyang

^ permalink raw reply

* [PATCH 0/3] Fix checkpatch errors in drivers/isdn/isdnloop
From: Bas Peters @ 2015-02-04 22:32 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, Bas Peters

This patchset adresses various checkpatch errors in the abovementioned driver.

Bas Peters (3):
  drivers: isdn: isdnloop: isdnloop.c: remove assignment of variables in
    if conditions,     in accordance with the CodingStyle.
  drivers: isdn: isdnloop: isdnloop.c: Fix brace positions according to 
       CodingStyle specifications.
  drivers: isdn: isdnloop: isdnloop.c: Remove parenthesis around return 
       values, as specified in CodingStyle.

 drivers/isdn/isdnloop/isdnloop.c | 64 +++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 34 deletions(-)

-- 
2.1.0

^ permalink raw reply

* [PATCH 1/3] drivers: isdn: isdnloop: isdnloop.c: remove assignment of variables in if conditions, in accordance with the CodingStyle.
From: Bas Peters @ 2015-02-04 22:32 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, Bas Peters
In-Reply-To: <1423089181-15850-1-git-send-email-baspeters93@gmail.com>

Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
 drivers/isdn/isdnloop/isdnloop.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index 5a4da94..af96317 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -59,7 +59,8 @@ isdnloop_bchan_send(isdnloop_card *card, int ch)
 	isdn_ctrl cmd;
 
 	while (card->sndcount[ch]) {
-		if ((skb = skb_dequeue(&card->bqueue[ch]))) {
+		skb = skb_dequeue(&card->bqueue[ch]);
+		if (skb) {
 			len = skb->len;
 			card->sndcount[ch] -= len;
 			ack = *(skb->head); /* used as scratch area */
@@ -317,7 +318,8 @@ isdnloop_polldchan(unsigned long data)
 	u_char *p;
 	isdn_ctrl cmd;
 
-	if ((skb = skb_dequeue(&card->dqueue)))
+	skb = skb_dequeue(&card->dqueue);
+	if (skb)
 		avail = skb->len;
 	else
 		avail = 0;
@@ -471,8 +473,8 @@ isdnloop_fake(isdnloop_card *card, char *s, int ch)
 {
 	struct sk_buff *skb;
 	int len = strlen(s) + ((ch >= 0) ? 3 : 0);
-
-	if (!(skb = dev_alloc_skb(len))) {
+	skb = dev_alloc_skb(len);
+	if (!skb) {
 		printk(KERN_WARNING "isdnloop: Out of memory in isdnloop_fake\n");
 		return 1;
 	}
@@ -1439,8 +1441,8 @@ isdnloop_initcard(char *id)
 {
 	isdnloop_card *card;
 	int i;
-
-	if (!(card = kzalloc(sizeof(isdnloop_card), GFP_KERNEL))) {
+	card = kzalloc(sizeof(isdnloop_card), GFP_KERNEL);
+	if (!card) {
 		printk(KERN_WARNING
 		       "isdnloop: (%s) Could not allocate card-struct.\n", id);
 		return (isdnloop_card *) 0;
@@ -1489,8 +1491,8 @@ static int
 isdnloop_addcard(char *id1)
 {
 	isdnloop_card *card;
-
-	if (!(card = isdnloop_initcard(id1))) {
+	card = isdnloop_initcard(id1);
+	if (!card) {
 		return -EIO;
 	}
 	printk(KERN_INFO
-- 
2.1.0

^ permalink raw reply related

* [PATCH 2/3] drivers: isdn: isdnloop: isdnloop.c: Fix brace positions according to CodingStyle specifications.
From: Bas Peters @ 2015-02-04 22:33 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, Bas Peters
In-Reply-To: <1423089181-15850-1-git-send-email-baspeters93@gmail.com>

 Signed-off-by: Bas Peters <baspeters93@gmail.com>

---
 drivers/isdn/isdnloop/isdnloop.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index af96317..e4d3620 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -150,8 +150,7 @@ typedef struct isdnloop_stat {
 	int action;
 } isdnloop_stat;
 /* *INDENT-OFF* */
-static isdnloop_stat isdnloop_stat_table[] =
-{
+static isdnloop_stat isdnloop_stat_table[] = {
 	{"BCON_",          ISDN_STAT_BCONN, 1}, /* B-Channel connected        */
 	{"BDIS_",          ISDN_STAT_BHUP,  2}, /* B-Channel disconnected     */
 	{"DCON_",          ISDN_STAT_DCONN, 0}, /* D-Channel connected        */
@@ -485,8 +484,7 @@ isdnloop_fake(isdnloop_card *card, char *s, int ch)
 	return 0;
 }
 /* *INDENT-OFF* */
-static isdnloop_stat isdnloop_cmd_table[] =
-{
+static isdnloop_stat isdnloop_cmd_table[] = {
 	{"BCON_R",         0,  1},	/* B-Channel connect        */
 	{"BCON_I",         0, 17},	/* B-Channel connect ind    */
 	{"BDIS_R",         0,  2},	/* B-Channel disconnect     */
@@ -527,10 +525,8 @@ isdnloop_fake_err(isdnloop_card *card)
 	isdnloop_fake(card, "NAK", -1);
 }
 
-static u_char ctable_eu[] =
-{0x00, 0x11, 0x01, 0x12};
-static u_char ctable_1t[] =
-{0x00, 0x3b, 0x01, 0x3a};
+static u_char ctable_eu[] = {0x00, 0x11, 0x01, 0x12};
+static u_char ctable_1t[] = {0x00, 0x3b, 0x01, 0x3a};
 
 /*
  * Assemble a simplified cause message depending on the
@@ -649,10 +645,8 @@ isdnloop_kill_ctimer(isdnloop_card *card, int ch)
 	spin_unlock_irqrestore(&card->isdnloop_lock, flags);
 }
 
-static u_char si2bit[] =
-{0, 1, 0, 0, 0, 2, 0, 4, 0, 0};
-static u_char bit2si[] =
-{1, 5, 7};
+static u_char si2bit[] = {0, 1, 0, 0, 0, 2, 0, 4, 0, 0};
+static u_char bit2si[] = {1, 5, 7};
 
 /*
  * Try finding a listener for an outgoing call.
-- 
2.1.0

^ permalink raw reply related

* [PATCH 3/3] drivers: isdn: isdnloop: isdnloop.c: Remove parenthesis around return values, as specified in CodingStyle.
From: Bas Peters @ 2015-02-04 22:33 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, Bas Peters
In-Reply-To: <1423089181-15850-1-git-send-email-baspeters93@gmail.com>

Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
 drivers/isdn/isdnloop/isdnloop.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/isdn/isdnloop/isdnloop.c b/drivers/isdn/isdnloop/isdnloop.c
index e4d3620..ef9c8e4 100644
--- a/drivers/isdn/isdnloop/isdnloop.c
+++ b/drivers/isdn/isdnloop/isdnloop.c
@@ -552,9 +552,9 @@ isdnloop_unicause(isdnloop_card *card, int loc, int cau)
 		sprintf(buf, "%02X44", ctable_1t[cau]);
 		break;
 	default:
-		return ("0000");
+		return "0000";
 	}
-	return (buf);
+	return buf;
 }
 
 /*
@@ -750,17 +750,17 @@ isdnloop_vstphone(isdnloop_card *card, char *phone, int caller)
 		if (caller) {
 			for (i = 0; i < 2; i++)
 				if (!(strcmp(card->s0num[i], phone)))
-					return (phone);
-			return (card->s0num[0]);
+					return phone;
+			return card->s0num[0];
 		}
-		return (phone);
+		return phone;
 		break;
 	case ISDN_PTYPE_1TR6:
 		if (caller) {
 			sprintf(nphone, "%s%c", card->s0num[0], phone[0]);
-			return (nphone);
+			return nphone;
 		} else
-			return (&phone[strlen(phone) - 1]);
+			return &phone[strlen(phone) - 1];
 		break;
 	}
 	return "";
@@ -1144,14 +1144,14 @@ isdnloop_command(isdn_ctrl *c, isdnloop_card *card)
 		case ISDNLOOP_IOCTL_STARTUP:
 			if (!access_ok(VERIFY_READ, (void *) a, sizeof(isdnloop_sdef)))
 				return -EFAULT;
-			return (isdnloop_start(card, (isdnloop_sdef *) a));
+			return isdnloop_start(card, (isdnloop_sdef *) a);
 			break;
 		case ISDNLOOP_IOCTL_ADDCARD:
 			if (copy_from_user((char *)&cdef,
 					   (char *)a,
 					   sizeof(cdef)))
 				return -EFAULT;
-			return (isdnloop_addcard(cdef.id1));
+			return isdnloop_addcard(cdef.id1);
 			break;
 		case ISDNLOOP_IOCTL_LEASEDCFG:
 			if (a) {
@@ -1373,7 +1373,7 @@ if_command(isdn_ctrl *c)
 	isdnloop_card *card = isdnloop_findcard(c->driver);
 
 	if (card)
-		return (isdnloop_command(c, card));
+		return isdnloop_command(c, card);
 	printk(KERN_ERR
 	       "isdnloop: if_command called with invalid driverId!\n");
 	return -ENODEV;
@@ -1387,7 +1387,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
 	if (card) {
 		if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 			return -ENODEV;
-		return (isdnloop_writecmd(buf, len, 1, card));
+		return isdnloop_writecmd(buf, len, 1, card);
 	}
 	printk(KERN_ERR
 	       "isdnloop: if_writecmd called with invalid driverId!\n");
@@ -1402,7 +1402,7 @@ if_readstatus(u_char __user *buf, int len, int id, int channel)
 	if (card) {
 		if (!(card->flags & ISDNLOOP_FLAGS_RUNNING))
 			return -ENODEV;
-		return (isdnloop_readstatus(buf, len, card));
+		return isdnloop_readstatus(buf, len, card);
 	}
 	printk(KERN_ERR
 	       "isdnloop: if_readstatus called with invalid driverId!\n");
@@ -1419,7 +1419,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
 			return -ENODEV;
 		/* ack request stored in skb scratch area */
 		*(skb->head) = ack;
-		return (isdnloop_sendbuf(channel, skb, card));
+		return isdnloop_sendbuf(channel, skb, card);
 	}
 	printk(KERN_ERR
 	       "isdnloop: if_sendbuf called with invalid driverId!\n");
@@ -1499,7 +1499,7 @@ static int __init
 isdnloop_init(void)
 {
 	if (isdnloop_id)
-		return (isdnloop_addcard(isdnloop_id));
+		return isdnloop_addcard(isdnloop_id);
 
 	return 0;
 }
-- 
2.1.0

^ permalink raw reply related

* [RFC: add openvswitch actions using BPF 0/2]
From: Andy Zhou @ 2015-02-04 22:48 UTC (permalink / raw)
  To: dev-yBygre7rU0SM8Zsap4Y0gw; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA

Joe and I have been experimenting with BPF and its application for OVS.
This patch shows our attempt to implement ovs actions using eBPF.

The kernel changes are against the 'net-next'. The corresponding
user space changes will be post next.

This patch set implements an BPF action, that has the same interface
as current OVS output action. Instead of sending out a packet, it
only generates a kernel message.

This feature is neither complete nor useful as is.  We are mostly
interested in comments on: The infrastructure changes to support
and running BPF functions, and suggestions on extensions beyond
those patches.

Andy Zhou (2):
  BPF: add a new BPF program type BPF_PROG_TYPE_OPENVSWITCH
  openvswitch: implements the BPF_PROG action in datapath

 include/linux/bpf.h              |  2 +-
 include/uapi/linux/bpf.h         |  1 +
 include/uapi/linux/openvswitch.h | 29 ++++++++++++-
 net/Makefile                     |  4 +-
 net/openvswitch/Makefile         |  2 +
 net/openvswitch/actions.c        | 30 +++++++++++++
 net/openvswitch/bpf.c            | 87 +++++++++++++++++++++++++++++++++++++
 net/openvswitch/datapath.c       |  6 ++-
 net/openvswitch/flow_netlink.c   | 92 +++++++++++++++++++++++++++++++++++++++-
 net/openvswitch/flow_netlink.h   |  8 ++++
 10 files changed, 254 insertions(+), 7 deletions(-)
 create mode 100644 net/openvswitch/bpf.c

-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply

* [RFC: add openvswitch actions using BPF 1/2] BPF: add a new BPF program type BPF_PROG_TYPE_OPENVSWITCH
From: Andy Zhou @ 2015-02-04 22:48 UTC (permalink / raw)
  To: dev-yBygre7rU0SM8Zsap4Y0gw; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1423090122-19807-1-git-send-email-azhou-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>

Add a new program type for openvswitch. Implements the BPF verifier
for the new type.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 include/linux/bpf.h              |  2 +-
 include/uapi/linux/bpf.h         |  1 +
 include/uapi/linux/openvswitch.h | 29 +++++++++++++-
 net/Makefile                     |  4 +-
 net/openvswitch/Makefile         |  2 +
 net/openvswitch/bpf.c            | 87 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 net/openvswitch/bpf.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index bbfceb7..2e71cc2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -99,7 +99,7 @@ enum bpf_access_type {
 
 struct bpf_verifier_ops {
 	/* return eBPF function prototype for verification */
-	const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
+	const struct bpf_func_proto *(*get_func_proto)(int func_id);
 
 	/* return true if 'size' wide access at offset 'off' within bpf_context
 	 * with 'type' (read or write) is allowed
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 45da7ec..a9a6b24 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -118,6 +118,7 @@ enum bpf_map_type {
 enum bpf_prog_type {
 	BPF_PROG_TYPE_UNSPEC,
 	BPF_PROG_TYPE_SOCKET_FILTER,
+	BPF_PROG_TYPE_OPENVSWITCH,
 };
 
 /* flags for BPF_MAP_UPDATE_ELEM command */
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 7a8785a..929999c 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -1,6 +1,6 @@
 
 /*
- * Copyright (c) 2007-2013 Nicira, Inc.
+ * Copyright (c) 2007-2015 Nicira, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of version 2 of the GNU General Public
@@ -569,6 +569,17 @@ struct ovs_action_push_vlan {
 	__be16 vlan_tci;	/* 802.1Q TCI (VLAN ID and priority). */
 };
 
+/**
+ * struct ovs_action_bpf_prog - %OVS_ACTION_ATTR_BPF_PROG action argument.
+ *
+ * XXX  The argument size is fixed for now.
+ */
+struct ovs_action_bpf_prog {
+	__be32 prog_fd;
+	__be32 arg0;
+	__be32 arg1;
+};
+
 /* Data path hash algorithm for computing Datapath hash.
  *
  * The algorithm type only specifies the fields in a flow
@@ -631,10 +642,26 @@ enum ovs_action_attr {
 	OVS_ACTION_ATTR_HASH,	      /* struct ovs_action_hash. */
 	OVS_ACTION_ATTR_PUSH_MPLS,    /* struct ovs_action_push_mpls. */
 	OVS_ACTION_ATTR_POP_MPLS,     /* __be16 ethertype. */
+	OVS_ACTION_ATTR_SET_MASKED,   /* place holder */
+	OVS_ACTION_ATTR_BPF_PROG,     /* strcut ovs_action_bpf_prog */
 
 	__OVS_ACTION_ATTR_MAX
 };
 
 #define OVS_ACTION_ATTR_MAX (__OVS_ACTION_ATTR_MAX - 1)
 
+/* integer value in 'imm' field of BPF_CALL instruction selects which OVS helper
+ * function eBPF program intends to call
+ */
+enum ovs_bpf_func_id {
+	OVS_BPF_FUNC_unspec,
+	OVS_BPF_FUNC_output,          /* int ovs_bpf_output(ctxt) */
+	__OVS_BPF_FUNC_MAX_ID,
+};
+
+struct ovs_bpf_action_ctxt {
+	void *skb;
+	u32  arg0;
+	u32  arg1;
+};
 #endif /* _LINUX_OPENVSWITCH_H */
diff --git a/net/Makefile b/net/Makefile
index 38704bd..7e92dea 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -67,7 +67,9 @@ obj-$(CONFIG_DNS_RESOLVER)	+= dns_resolver/
 obj-$(CONFIG_CEPH_LIB)		+= ceph/
 obj-$(CONFIG_BATMAN_ADV)	+= batman-adv/
 obj-$(CONFIG_NFC)		+= nfc/
-obj-$(CONFIG_OPENVSWITCH)	+= openvswitch/
+ifneq ($(CONFIG_OPENVSWITCH),)
+obj-y				+= openvswitch/
+endif
 obj-$(CONFIG_VSOCKETS)	+= vmw_vsock/
 obj-$(CONFIG_NET_MPLS_GSO)	+= mpls/
 obj-$(CONFIG_HSR)		+= hsr/
diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
index 91b9478..5a3a2b7 100644
--- a/net/openvswitch/Makefile
+++ b/net/openvswitch/Makefile
@@ -18,3 +18,5 @@ openvswitch-y := \
 obj-$(CONFIG_OPENVSWITCH_GENEVE)+= vport-geneve.o
 obj-$(CONFIG_OPENVSWITCH_VXLAN)	+= vport-vxlan.o
 obj-$(CONFIG_OPENVSWITCH_GRE)	+= vport-gre.o
+
+obj-y += bpf.o
diff --git a/net/openvswitch/bpf.c b/net/openvswitch/bpf.c
new file mode 100644
index 0000000..8a33e93
--- /dev/null
+++ b/net/openvswitch/bpf.c
@@ -0,0 +1,87 @@
+/* Copyright (c) 2015 Nicira Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/err.h>
+#include <linux/bpf.h>
+#include <linux/openvswitch.h>
+#include <linux/skbuff.h>
+
+static u64 bpf_helper_output(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	struct sk_buff *skb = (struct sk_buff *) (unsigned long) r1;
+	uint32_t port = (uint32_t) (unsigned long) r2;
+
+	printk("helper output %p to port %d\n", skb, port);
+	return 0;
+}
+
+struct bpf_func_proto bpf_helper_output_proto = {
+	.func = bpf_helper_output,
+	.gpl_only = true,
+	.ret_type = RET_INTEGER,
+	.arg1_type = ARG_ANYTHING,  /* XXX from context */
+	.arg2_type = ARG_ANYTHING,
+	.arg3_type = ARG_ANYTHING,
+	.arg4_type = ARG_ANYTHING,
+};
+
+#define BPF_CONTEXT_ACCESS(CTXT, FIELD, RW) \
+	[offsetof(struct CTXT, FIELD)] = { \
+		FIELD_SIZEOF(struct CTXT, FIELD), \
+		RW  \
+	}
+
+static const struct bpf_func_proto *ovs_func_proto(int func_id)
+{
+	switch (func_id) {
+	case OVS_BPF_FUNC_output:
+		return &bpf_helper_output_proto;
+	default:
+		return NULL;
+	}
+}
+
+static const struct bpf_context_access {
+	int size;
+	enum bpf_access_type type;
+} bpf_ctx_access[] = {
+	BPF_CONTEXT_ACCESS(ovs_bpf_action_ctxt, skb, BPF_READ),
+	BPF_CONTEXT_ACCESS(ovs_bpf_action_ctxt, arg0, BPF_READ),
+	BPF_CONTEXT_ACCESS(ovs_bpf_action_ctxt, arg1, BPF_READ)
+};
+
+static bool test_is_valid_access(int off, int size, enum bpf_access_type type)
+{
+	const struct bpf_context_access *access;
+
+	if (off < 0 || off >= ARRAY_SIZE(bpf_ctx_access))
+		return false;
+
+	access = &bpf_ctx_access[off];
+	if (access->size == size && (access->type & type))
+		return true;
+
+	return false;
+}
+
+static struct bpf_verifier_ops ovs_bpf_ops = {
+	.get_func_proto = ovs_func_proto,
+	.is_valid_access = test_is_valid_access,
+};
+
+static struct bpf_prog_type_list tl_prog = {
+	.ops = &ovs_bpf_ops,
+	.type = BPF_PROG_TYPE_OPENVSWITCH,
+};
+
+static int __init register_ovs_bpf_ops(void)
+{
+	bpf_register_prog_type(&tl_prog);
+	return 0;
+}
+late_initcall(register_ovs_bpf_ops);
-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply related

* [RFC: add openvswitch actions using BPF 2/2] openvswitch: implements the BPF_PROG action in datapath
From: Andy Zhou @ 2015-02-04 22:48 UTC (permalink / raw)
  To: dev-yBygre7rU0SM8Zsap4Y0gw; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1423090122-19807-1-git-send-email-azhou-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>

BPF_PROG action allows an action to be implemented in eBPF language and
downloaded by the userspace at runtime.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 net/openvswitch/actions.c      | 30 ++++++++++++++
 net/openvswitch/datapath.c     |  6 ++-
 net/openvswitch/flow_netlink.c | 92 +++++++++++++++++++++++++++++++++++++++++-
 net/openvswitch/flow_netlink.h |  8 ++++
 4 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index b4cffe6..29e9171 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -38,8 +38,12 @@
 
 #include "datapath.h"
 #include "flow.h"
+#include "flow_netlink.h"
 #include "vport.h"
 
+typedef int (*ovs_bpf_func_t)(const struct ovs_bpf_action_ctxt *,
+			      const struct bpf_insn *);
+
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 			      struct sw_flow_key *key,
 			      const struct nlattr *attr, int len);
@@ -747,6 +751,28 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
 	return 0;
 }
 
+static int execute_bpf(struct sk_buff *skb, struct sw_flow_key *key,
+		       const struct nlattr *a)
+{
+	struct ovs_action_bpf_runtime *rt;
+	struct bpf_prog *prog;
+	struct ovs_bpf_action_ctxt ctxt;
+	ovs_bpf_func_t ovs_bpf_func;
+	int err;
+
+	rt = nla_data(a);
+	prog = rt->prog;
+
+	/* Build the BPF program runtime context. */
+	ctxt.skb = (void *)skb;
+	ctxt.arg0 = rt->arg0;
+	ctxt.arg1 = rt->arg1;
+
+	ovs_bpf_func = (ovs_bpf_func_t)(prog->bpf_func);
+	err = ovs_bpf_func(&ctxt, prog->insnsi);
+	return err;
+}
+
 /* Execute a list of actions against 'skb'. */
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 			      struct sw_flow_key *key,
@@ -814,6 +840,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 			}
 			break;
 
+		case OVS_ACTION_ATTR_BPF_PROG:
+			err = execute_bpf(skb, key, a);
+			break;
+
 		case OVS_ACTION_ATTR_SET:
 			err = execute_set_action(skb, key, nla_data(a));
 			break;
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ae5e77c..810a0bf 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -699,6 +699,8 @@ static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts,
 		len += nla_total_size(ovs_key_attr_size());
 
 	/* OVS_FLOW_ATTR_ACTIONS */
+	/* XXX this logic needs to be fixed to accommodate BPF_PROG action
+	 * will expand the run time action size.   */
 	if (should_fill_actions(ufid_flags))
 		len += nla_total_size(acts->actions_len);
 
@@ -1017,7 +1019,7 @@ err_unlock_ovs:
 	ovs_unlock();
 	kfree_skb(reply);
 err_kfree_acts:
-	kfree(acts);
+	free_flow_actions(acts);
 err_kfree_flow:
 	ovs_flow_free(new_flow, false);
 error:
@@ -1152,7 +1154,7 @@ err_unlock_ovs:
 	ovs_unlock();
 	kfree_skb(reply);
 err_kfree_acts:
-	kfree(acts);
+	free_flow_actions(acts);
 error:
 	return error;
 }
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 8b9a612..466e85f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -42,6 +42,7 @@
 #include <linux/icmp.h>
 #include <linux/icmpv6.h>
 #include <linux/rculist.h>
+#include <linux/bpf.h>
 #include <net/geneve.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
@@ -1546,11 +1547,38 @@ static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
 	return sfa;
 }
 
+void free_flow_actions(struct sw_flow_actions *sf_acts)
+{
+	const struct nlattr *a;
+	int rem;
+	struct ovs_action_bpf_runtime *rt;
+
+	nla_for_each_attr(a, sf_acts->actions, sf_acts->actions_len, rem) {
+		int type = nla_type(a);
+
+		switch (type) {
+		case OVS_ACTION_ATTR_BPF_PROG:
+			rt = nla_data(a);
+			bpf_prog_put(rt->prog);
+			break;
+		}
+	}
+
+	kfree(sf_acts);
+}
+
+static void free_flow_actions_rcu(struct rcu_head *head)
+{
+	struct sw_flow_actions *acts = container_of(head, struct sw_flow_actions, rcu);
+
+	free_flow_actions(acts);
+}
+
 /* Schedules 'sf_acts' to be freed after the next RCU grace period.
  * The caller must hold rcu_read_lock for this to be sensible. */
 void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts)
 {
-	kfree_rcu(sf_acts, rcu);
+	call_rcu(&sf_acts->rcu, free_flow_actions_rcu);
 }
 
 static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa,
@@ -1695,6 +1723,37 @@ static int validate_and_copy_sample(const struct nlattr *attr,
 	return 0;
 }
 
+static int validate_and_copy_bpf(const struct nlattr *attr,
+				 struct sw_flow_actions **sfa,
+				 bool log)
+{
+	const struct ovs_action_bpf_prog *act_bpf = nla_data(attr);
+	struct ovs_action_bpf_runtime rt;
+	u32 fd;
+	int err;
+
+	fd = ntohl(act_bpf->prog_fd);
+	rt.prog = bpf_prog_get(fd);
+	if (!rt.prog)
+		return -EINVAL;
+
+	if (rt.prog->aux->prog_type != BPF_PROG_TYPE_OPENVSWITCH) {
+		bpf_prog_put(rt.prog);
+		return -EINVAL;
+	}
+
+	rt.fd = fd;
+	rt.arg0 = ntohl(act_bpf->arg0);
+	rt.arg1 = ntohl(act_bpf->arg1);
+
+	/* Validation done.  Rewrite the action with runtime datastructure. */
+	err = add_action(sfa, OVS_ACTION_ATTR_BPF_PROG, &rt, sizeof(rt), log);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int validate_tp_port(const struct sw_flow_key *flow_key,
 			    __be16 eth_type)
 {
@@ -1966,7 +2025,9 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 			[OVS_ACTION_ATTR_POP_VLAN] = 0,
 			[OVS_ACTION_ATTR_SET] = (u32)-1,
 			[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
-			[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
+			[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash),
+			[OVS_ACTION_ATTR_BPF_PROG] =
+				sizeof(struct ovs_action_bpf_prog),
 		};
 		const struct ovs_action_push_vlan *vlan;
 		int type = nla_type(a);
@@ -2073,6 +2134,13 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 			skip_copy = true;
 			break;
 
+		case OVS_ACTION_ATTR_BPF_PROG:
+			err = validate_and_copy_bpf(a, sfa, log);
+			if (err)
+				return err;
+			skip_copy = true;
+			break;
+
 		default:
 			OVS_NLERR(log, "Unknown Action type %d", type);
 			return -EINVAL;
@@ -2177,6 +2245,21 @@ static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb)
 	return 0;
 }
 
+static int bpf_action_to_attr(const struct nlattr *a, struct sk_buff *skb)
+{
+	const struct ovs_action_bpf_runtime *rt = nla_data(a);
+	struct ovs_action_bpf_prog prog;
+
+	prog.prog_fd = htonl(rt->fd);
+	prog.arg0 = htonl(rt->arg0);
+	prog.arg1 = htonl(rt->arg1);
+
+	if (nla_put(skb, OVS_ACTION_ATTR_BPF_PROG, sizeof(prog), &prog));
+		return -EMSGSIZE;
+
+	return 0;
+}
+
 int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
 {
 	const struct nlattr *a;
@@ -2197,6 +2280,11 @@ int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
 			if (err)
 				return err;
 			break;
+
+		case OVS_ACTION_ATTR_BPF_PROG:
+			err = bpf_action_to_attr(a, skb);
+			break;
+
 		default:
 			if (nla_put(skb, type, nla_len(a), nla_data(a)))
 				return -EMSGSIZE;
diff --git a/net/openvswitch/flow_netlink.h b/net/openvswitch/flow_netlink.h
index 5c3d75b..e986ddb 100644
--- a/net/openvswitch/flow_netlink.h
+++ b/net/openvswitch/flow_netlink.h
@@ -68,6 +68,14 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
 int ovs_nla_put_actions(const struct nlattr *attr,
 			int len, struct sk_buff *skb);
 
+void free_flow_actions(struct sw_flow_actions *);
 void ovs_nla_free_flow_actions(struct sw_flow_actions *);
 
+struct ovs_action_bpf_runtime {
+	uint32_t fd;
+	uint32_t arg0;
+	uint32_t arg1;
+	struct bpf_prog *prog;
+};
+
 #endif /* flow_netlink.h */
-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply related

* [RFC: add openvswitch actions using BPF 0/9]
From: Andy Zhou @ 2015-02-04 22:49 UTC (permalink / raw)
  To: dev-yBygre7rU0SM8Zsap4Y0gw; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA

User space changes of BPF OVS action agains curret OVS master.

Andy Zhou (9):
  hack: Do not compile datapath
  odp: add a new ODP action: OVS_ACTION_ATTR_BPF_PROG
  tests: add a OVS_ACTION_ATTR_BPF_PROG ation unit test case
  autoconf: support -with-llc options
  bpf: add the first BPF program.
  lib: import into libbpf to ovs/lib
  ofproto-dpif: Add eBPF program loader and runtime infrasturcure.
  ofproto-dpif: Add datapath eBPF support detection
  ofproto-dpif-xlate: generate BPF output action (Hack)

 INSTALL.BPF.md                                    |  42 ++
 Makefile.am                                       |   5 +-
 acinclude.m4                                      |  18 +-
 bpf/automake.mk                                   |  27 ++
 bpf/bpf-shared.h                                  |  12 +
 bpf/ovs-actions.c                                 |  13 +
 bpf/ovs-bpf-helpers.h                             |  35 ++
 configure.ac                                      |   2 +
 datapath/linux/compat/include/linux/openvswitch.h |  29 +-
 lib/automake.mk                                   |  22 ++
 lib/dpif-netdev.c                                 |   1 +
 lib/dpif.c                                        |   1 +
 lib/libbpf.c                                      | 100 +++++
 lib/libbpf.h                                      | 185 +++++++++
 lib/odp-execute.c                                 |   1 +
 lib/odp-util.c                                    |  33 ++
 ofproto/automake.mk                               |   2 +
 ofproto/ofproto-dpif-bpf.c                        | 454 ++++++++++++++++++++++
 ofproto/ofproto-dpif-bpf.h                        |  42 ++
 ofproto/ofproto-dpif-xlate.c                      |  18 +-
 ofproto/ofproto-dpif.c                            |  21 +
 tests/odp.at                                      |   1 +
 vswitchd/automake.mk                              |   5 +
 23 files changed, 1064 insertions(+), 5 deletions(-)
 create mode 100644 INSTALL.BPF.md
 create mode 100644 bpf/automake.mk
 create mode 100644 bpf/bpf-shared.h
 create mode 100644 bpf/ovs-actions.c
 create mode 100644 bpf/ovs-bpf-helpers.h
 create mode 100644 lib/libbpf.c
 create mode 100644 lib/libbpf.h
 create mode 100644 ofproto/ofproto-dpif-bpf.c
 create mode 100644 ofproto/ofproto-dpif-bpf.h

-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox