* Re: [PATCH net-next] bnx2x: Fix static checker warning regarding `txdata_ptr'
From: David Miller @ 2014-08-27 23:29 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev, kernel-janitors, Ariel.Elior, dan.carpenter
In-Reply-To: <1409037881-4192-1-git-send-email-Yuval.Mintz@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@qlogic.com>
Date: Tue, 26 Aug 2014 10:24:41 +0300
> Incorrect checking of array instead of array contents in panic_dump
> flow - results of commit e261199872a2 ("bnx2x: Safe bnx2x_panic_dump()").
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH (net.git) 3/4] stmmac: never check EEE in case of a switch is attached
From: David Miller @ 2014-08-27 23:28 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1409037383-3213-4-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Tue, 26 Aug 2014 09:16:22 +0200
> From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
>
> This patch is to skip the EEE initialisation when the stmmac
> is using a switch (with a fixed phy support).
>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 51a89d4..48a112f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -276,6 +276,7 @@ static void stmmac_eee_ctrl_timer(unsigned long arg)
> bool stmmac_eee_init(struct stmmac_priv *priv)
> {
> bool ret = false;
> + char *phy_bus_name = priv->plat->phy_bus_name;
>
> /* Using PCS we cannot dial with the phy registers at this stage
> * so we do not support extra feature like EEE.
Please order local variable declarations in reverse-christmas-tree order,
this means longer lines come before shorter ones.
Thanks.
^ permalink raw reply
* Re: [PATCH (net.git)] stmmac: ptp: fix the reference clock
From: David Miller @ 2014-08-27 23:26 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1409033072-10508-1-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Tue, 26 Aug 2014 08:04:32 +0200
> @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
> if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
> return -EOPNOTSUPP;
>
> + /* Fall-back to main clock in case of no PTP ref is passed */
> + priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref");
> + if (IS_ERR(priv->clk_ptp_ref)) {
> + priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk);
This code path will leave an IS_ERR() value in clk_ptp_ref.
...
> static void stmmac_release_ptp(struct stmmac_priv *priv)
> {
> + if (priv->clk_ptp_ref)
> + clk_disable_unprepare(priv->clk_ptp_ref);
> stmmac_ptp_unregister(priv);
> }
Which this code will erroneously pass into clk_disable_unprepare().
Please NULL the pointer out properl when devm_clk_get() errors.
Thanks.
^ permalink raw reply
* Re: [PATCH] Driver for IBM System i/p VNIC protocol
From: David Miller @ 2014-08-27 23:25 UTC (permalink / raw)
To: santil; +Cc: netdev
In-Reply-To: <53FB3FA8.3040304@linux.vnet.ibm.com>
From: Santiago Leon <santil@linux.vnet.ibm.com>
Date: Mon, 25 Aug 2014 08:52:40 -0500
> This is a new device driver for a high performance SR-IOV assisted virtual
> network for IBM System p and IBM System i systems. The SR-IOV VF will be
> attached to the VIOS partition and mapped to the Linux client via the
> hypervisor's VNIC protocol that this driver implements.
>
> This driver is able to perform basic tx and rx, new features and improvements
> will be added as they are being developed and tested.
>
>
> Signed-off-by: Santiago Leon <santil@linux.vnet.ibm.com>
Just a heads up, if nobody reviews this driver submission I am not
applying it.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 01/13] net: dsa: reduce number of protocol hooks
From: David Miller @ 2014-08-27 23:24 UTC (permalink / raw)
To: alexander.h.duyck; +Cc: f.fainelli, netdev, linville, jhs, alexander.duyck
In-Reply-To: <53FDF879.1000504@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Wed, 27 Aug 2014 08:25:45 -0700
> Just a thought, you might want to consider using a protocol value other
> than ETH_P_DSA. Maybe we should add a new value named ETH_P_XDSA for
> this approach since what we are doing is a multiplexed DSA protocol now.
>
> This would allow for much easier back-porting of this if needed and also
> prevents us from possibly triggering any issues if there are any
> user-space APIs that might be trying to capture packets based on the
> protocol value.
Agreed, Florian can you respin this patch set using a newly allocated
ETH_P_* value for these new semantics?
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] r8152: replace strncpy with strlcpy
From: David Miller @ 2014-08-27 23:18 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-22-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Tue, 26 Aug 2014 10:08:23 +0800
> Replace the strncpy with strlcpy, and use sizeof to determine the
> length.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next] ipv6: stop sending PTB packets for MTU < 1280
From: Hannes Frederic Sowa @ 2014-08-27 23:07 UTC (permalink / raw)
To: Fernando Gont; +Cc: Hagen Paul Pfeifer, netdev
In-Reply-To: <53FE4086.8040708@si6networks.com>
Hi Fernando,
On Mi, 2014-08-27 at 17:33 -0300, Fernando Gont wrote:
> On 08/25/2014 07:47 PM, Hannes Frederic Sowa wrote:
> > Hi Hagen,
> >
> > On Di, 2014-08-26 at 00:25 +0200, Hagen Paul Pfeifer wrote:
> >> Reduce the attack vector and stop generating ICMPv6 packet to big for
> >> packets smaller then the minimal required IPv6 MTU.
> >>
> >> See
> >> http://tools.ietf.org/html/draft-gont-6man-deprecate-atomfrag-generation-00
> >
> > I wonder if we should wait until this gets RFC status?
> >
> > I very much welcome this decision! I already raised this problem some
> > time ago:
> > http://lists.openwall.net/netdev/2013/12/31/17
>
> FWIW, this issue you reported is related, but different from the one
> I've described. The one I've described is based on sending ICMPv6
> PTB<1280. RFC2460 states that when you receive an ICMPv6 PTB<1280 you
> should add a Fragment Header to all packets sent to that destination
> (i.e., produce the so called "IPv6 atomic fragments").
>
> These "atomic fragments" have an offset=0, and MF=0 -- i.e., they are
> not really fragmented.
Sure, in this specific thread I was mostly concerned with DNS packet
blackholing in forwarding path. Because of DNSSEC packets also easily
getting bigger than 1280 bytes this effect could also hurt people
without dst_allfrag being true (the function which checks if atomic
fragments are in effect along a route because of a <1280 PtB
notification before). At that time, I had atomic fragments in mind, they
just weren't that much of a problem as we are not concerned about them
in forwarding path and don't check for dst_allfrag there. But we also
had other changes to harden the host side in regard to PtB acceptance,
specifically this also helps to harden against spoofed PtB with mtu <
1280, see below.
At former times Linux used path mtu information in the forwarding path,
so not only the end system would be vulnerable, but all systems along
the way if one could sneak in a PtB error to reduce path mtu. This could
get used to inject PtB notifications even into full quadruple looked up
connections by just letting the router returning the PtB error instead
of directly spoofing it. At least there is no way you could generate
PtBs with mtus smaller than 1280, so no problems in regard to atomic
fragments.
You still had to ensure to hit a live socket, though. On DNS servers
with UDP this could be easily done because on unconnected sockets we
only do a 2-tuple lookup on the socket to validate the PtB error, so it
was easily possible to hit unconnected DNS servers ports on a router.
For protection on the host side:
In case you have an IPv6 socket and you want to ensure that no PtB
information will be accepted by it you already can set IP_MTU_DISCOVER
to IP_PMTUDISC_OMIT (or to IP_PMTUDISC_INTERFACE but kernel will then
start to reject creating fragments at all and will signal error to user
space). This is already implemented in unbound and hopefully already
found its way into BIND to protect DNS.
> Hence the trivial way to mitigate this attack is to drop incoming ICMPv6
> PTB1280 (or, at the very least, don't react to them by sending all
> subsequent packets with a Fragment Header).
Yes, atomic fragments aggravate this problem.
In retrospect, I don't know about other operationg systems, at that time
I only checked Linux and FreeBSD:
FreeBSD only uses interface MTU to check if PtB should be generated in
forwarding path.
Linux used to check path mtu but I converted that to checks for
interface mtu only in fwd path, too.
FreeBSD only accepts PtB information for TCP connections where the inner
data of the ICMP error matches the full quadruple of a tcp connected
socket. FreeBSD also validates that the ICMP(v6) is in the TCP window.
FreeBSD does not deal with PtB information on UDP sockets at all!
Linux does the same for TCP sockets (also checks windowing information).
But Linux also accepts 2-tuple UDP PtB errors (unconnected sockets),
this is now configurable on a per socket basis but we don't have a way
to turn this mode on globally as this is possible for IPv4.
That said, it is not that easy to forge PtB information any more. Only
systems where unconnected UDP sockets are exposed may be easily attacked
(but this is still a big enough attack vector). On TCP connections one
would need to spray a lot of packets against those hosts to match full
quadruple and get into the TCP window.
Bye,
Hannes
^ permalink raw reply
* Re: [patch net-next RFC 03/12] net: introduce generic switch devices support
From: Cong Wang @ 2014-08-27 22:19 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Neil Horman, Andy Gospodarek, Thomas Graf,
Daniel Borkmann, Or Gerlitz, Jesse Gross, Pravin B Shelar, azhou,
Ben Hutchings, Stephen Hemminger, jeffrey.t.kirsher, vyasevic,
Cong Wang, john.r.fastabend, Eric Dumazet, Jamal Hadi Salim,
Scott Feldman, f.fainelli, roopa, linville, dev, jasowang,
Eric W. Biederman, Nicolas Dichtel, ryazanov.s.a
In-Reply-To: <1408637945-10390-4-git-send-email-jiri@resnulli.us>
On Thu, Aug 21, 2014 at 9:18 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> diff --git a/include/linux/switchdev.h b/include/linux/switchdev.h
> new file mode 100644
> index 0000000..ba77a68
> --- /dev/null
> +++ b/include/linux/switchdev.h
It should be in include/net/ instead, since it never
goes out of networking.
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: Clarification of CHECKSUM_UNNECESSARY
From: David Miller @ 2014-08-27 21:40 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <CA+mtBx9GvHpLJ718hV5BjRGjfWvmXB55hKa3Rwo=6me3f0M3Dw@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Mon, 25 Aug 2014 18:47:35 -0700
> On Mon, Aug 25, 2014 at 6:40 PM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Aug 25, 2014 at 6:13 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Tom Herbert <therbert@google.com>
>>> Date: Mon, 25 Aug 2014 17:55:56 -0700 (PDT)
>>>
>>>> @@ -569,16 +590,13 @@ struct sk_buff {
>>>> __u8 wifi_acked:1;
>>>> __u8 no_fcs:1;
>>>> __u8 head_frag:1;
>>>> - /* Encapsulation protocol and NIC drivers should use
>>>> - * this flag to indicate to each other if the skb contains
>>>> - * encapsulated packet or not and maybe use the inner packet
>>>> - * headers if needed
>>>> - */
>>>> + /* Indicates the the inner headers are valid in the skbuff. */
>>>> __u8 encapsulation:1;
>>>> __u8 encap_hdr_csum:1;
>>>> __u8 csum_valid:1;
>>>> __u8 csum_complete_sw:1;
>>>> - /* 2/4 bit hole (depending on ndisc_nodetype presence) */
>>>> + __u8 csum_level:2;
>>>> + /* 0/2 bit hole (depending on ndisc_nodetype presence) */
>>>> kmemcheck_bitfield_end(flags2);
>>>
>>> Crap, with xmit_more, this actually bleeds us over into a new __u8.
>>
>> We'll undoubtably want to add more flags beyond that (they're quite
>> useful). I'll try to find some more space in the existing fields so we
>> don't increase skbuf size.
>
> Actually, from inner_protocol through mac_header there are seven 16
> bit fields, could put another 16 bit flags there.
Ok, please resubmit this series with that change.
Meanwhile I fixed up the comment so that it's accurate again, like so:
====================
Subject: [PATCH] net: Update sk_buff flag bit availability comment.
We lost one when xmit_more was added.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/linux/skbuff.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9b3802a..b69b7b5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -580,7 +580,7 @@ struct sk_buff {
__u8 encap_hdr_csum:1;
__u8 csum_valid:1;
__u8 csum_complete_sw:1;
- /* 2/4 bit hole (depending on ndisc_nodetype presence) */
+ /* 1/3 bit hole (depending on ndisc_nodetype presence) */
kmemcheck_bitfield_end(flags2);
#if defined CONFIG_NET_DMA || defined CONFIG_NET_RX_BUSY_POLL
--
1.7.11.7
^ permalink raw reply related
* Re: [RFC PATCH] pktgen: skb bursting via skb->xmit_more API
From: David Miller @ 2014-08-27 21:36 UTC (permalink / raw)
To: brouer; +Cc: netdev, dborkman, hannes, cwang
In-Reply-To: <20140827211300.26976.52104.stgit@dragon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Wed, 27 Aug 2014 23:13:00 +0200
> This patch just demonstrates the effect of delaying the HW tailptr,
> the skb->xmit_more API should likely have some wrappers.
>
> One issue is the possible need to flush/write the tailptr on
> the exit path... marked with FIXME.
...
> Performance
> skb_burst=0 tx:5614370 pps
> skb_burst=1 tx:5571279 pps ( -1.38 ns (worse))
> skb_burst=2 tx:6942821 pps ( 35.46 ns)
> skb_burst=3 tx:7556214 pps ( 11.69 ns)
> skb_burst=4 tx:7740632 pps ( 3.15 ns)
> skb_burst=5 tx:7972489 pps ( 3.76 ns)
> skb_burst=6 tx:8129856 pps ( 2.43 ns)
> skb_burst=7 tx:8281671 pps ( 2.25 ns)
> skb_burst=8 tx:8383790 pps ( 1.47 ns)
> skb_burst=9 tx:8451248 pps ( 0.95 ns)
> skb_burst=10 tx:8503571 pps ( 0.73 ns)
> skb_burst=16 tx:8745878 pps ( 3.26 ns)
> skb_burst=24 tx:8871629 pps ( 1.62 ns)
> skb_burst=32 tx:8945166 pps ( 0.93 ns)
>
> skb_burst=(0 vs 32) improvement:
> (1/5614370*10^9)-(1/8945166*10^9) = 66.32 ns
> + 3330796 pps
Thanks for doing these tests Jesper.
^ permalink raw reply
* [RFC PATCH] pktgen: skb bursting via skb->xmit_more API
From: Jesper Dangaard Brouer @ 2014-08-27 21:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Daniel Borkmann,
Hannes Frederic Sowa
Cc: cwang
This patch just demonstrates the effect of delaying the HW tailptr,
the skb->xmit_more API should likely have some wrappers.
One issue is the possible need to flush/write the tailptr on
the exit path... marked with FIXME.
Let me demonstrate the performance effect of bulking packet with pktgen.
These results is a **single** CPU pktgen TX via script:
https://github.com/netoptimizer/network-testing/blob/master/pktgen/pktgen02_burst.sh
Cmdline args:
./pktgen02_burst.sh -i eth5 -d 192.168.21.4 -m 00:12:c0:80:1d:54 -b $skb_burst
Special case skb_burst=1 does not burst, but activates the
skb_burst_count++ and writing to skb->xmit_more.
Performance
skb_burst=0 tx:5614370 pps
skb_burst=1 tx:5571279 pps ( -1.38 ns (worse))
skb_burst=2 tx:6942821 pps ( 35.46 ns)
skb_burst=3 tx:7556214 pps ( 11.69 ns)
skb_burst=4 tx:7740632 pps ( 3.15 ns)
skb_burst=5 tx:7972489 pps ( 3.76 ns)
skb_burst=6 tx:8129856 pps ( 2.43 ns)
skb_burst=7 tx:8281671 pps ( 2.25 ns)
skb_burst=8 tx:8383790 pps ( 1.47 ns)
skb_burst=9 tx:8451248 pps ( 0.95 ns)
skb_burst=10 tx:8503571 pps ( 0.73 ns)
skb_burst=16 tx:8745878 pps ( 3.26 ns)
skb_burst=24 tx:8871629 pps ( 1.62 ns)
skb_burst=32 tx:8945166 pps ( 0.93 ns)
skb_burst=(0 vs 32) improvement:
(1/5614370*10^9)-(1/8945166*10^9) = 66.32 ns
+ 3330796 pps
---
net/core/pktgen.c | 34 +++++++++++++++++++++++++++++++++-
1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 83e2b4b..ac5f7c4 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -269,6 +269,8 @@ struct pktgen_dev {
__u64 allocated_skbs;
__u32 clone_count;
+
+ int skb_burst_count; /* counter for skb_burst */
int last_ok; /* Was last skb sent?
* Or a failed transmit of some sort?
* This will keep sequence numbers in order
@@ -386,6 +388,9 @@ struct pktgen_dev {
u16 queue_map_min;
u16 queue_map_max;
__u32 skb_priority; /* skb priority field */
+ int skb_burst; /* Bursting SKBs by delaying HW
+ * tailptr via skb->xmit_more
+ */
int node; /* Memory node */
#ifdef CONFIG_XFRM
@@ -612,6 +617,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
if (pkt_dev->traffic_class)
seq_printf(seq, " traffic_class: 0x%02x\n", pkt_dev->traffic_class);
+ if (pkt_dev->skb_burst)
+ seq_printf(seq, " skb_burst: %d\n", pkt_dev->skb_burst);
+
if (pkt_dev->node >= 0)
seq_printf(seq, " node: %d\n", pkt_dev->node);
@@ -1120,6 +1128,16 @@ static ssize_t pktgen_if_write(struct file *file,
pkt_dev->dst_mac_count);
return count;
}
+ if (!strcmp(name, "skb_burst")) {
+ len = num_arg(&user_buffer[i], 10, &value);
+ if (len < 0)
+ return len;
+
+ i += len;
+ pkt_dev->skb_burst = value;
+ sprintf(pg_result, "OK: skb_burst=%d", pkt_dev->skb_burst);
+ return count;
+ }
if (!strcmp(name, "node")) {
len = num_arg(&user_buffer[i], 10, &value);
if (len < 0)
@@ -3165,6 +3183,7 @@ static int pktgen_stop_device(struct pktgen_dev *pkt_dev)
return -EINVAL;
}
+ // FIXME: Possibly missing a tailptr flush here...
pkt_dev->running = 0;
kfree_skb(pkt_dev->skb);
pkt_dev->skb = NULL;
@@ -3327,6 +3346,16 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
queue_map = skb_get_queue_mapping(pkt_dev->skb);
txq = netdev_get_tx_queue(odev, queue_map);
+ /* Do HW level bursting via skb->xmit_more */
+ if (pkt_dev->skb_burst > 0) {
+ if (pkt_dev->skb_burst_count++ < pkt_dev->skb_burst) {
+ pkt_dev->skb->xmit_more = 1;
+ } else {
+ pkt_dev->skb->xmit_more = 0;
+ pkt_dev->skb_burst_count = 1;
+ }
+ }
+
local_bh_disable();
HARD_TX_LOCK(odev, txq, smp_processor_id());
@@ -3337,7 +3366,8 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
goto unlock;
}
atomic_inc(&(pkt_dev->skb->users));
- ret = netdev_start_xmit(pkt_dev->skb, odev);
+ ret = odev->netdev_ops->ndo_start_xmit(pkt_dev->skb, odev);
+ //ret = netdev_start_xmit(pkt_dev->skb, odev);
switch (ret) {
case NETDEV_TX_OK:
@@ -3562,6 +3592,8 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
pkt_dev->svlan_p = 0;
pkt_dev->svlan_cfi = 0;
pkt_dev->svlan_id = 0xffff;
+ pkt_dev->skb_burst = 0;
+ pkt_dev->skb_burst_count = 1;
pkt_dev->node = -1;
err = pktgen_setup_dev(t->net, pkt_dev, ifname);
^ permalink raw reply related
* Re: [RFC PATCH net-next] ipv6: stop sending PTB packets for MTU < 1280
From: Fernando Gont @ 2014-08-27 20:33 UTC (permalink / raw)
To: Hannes Frederic Sowa, Hagen Paul Pfeifer; +Cc: netdev
In-Reply-To: <1409006842.6274.69.camel@localhost>
On 08/25/2014 07:47 PM, Hannes Frederic Sowa wrote:
> Hi Hagen,
>
> On Di, 2014-08-26 at 00:25 +0200, Hagen Paul Pfeifer wrote:
>> Reduce the attack vector and stop generating ICMPv6 packet to big for
>> packets smaller then the minimal required IPv6 MTU.
>>
>> See
>> http://tools.ietf.org/html/draft-gont-6man-deprecate-atomfrag-generation-00
>
> I wonder if we should wait until this gets RFC status?
>
> I very much welcome this decision! I already raised this problem some
> time ago:
> http://lists.openwall.net/netdev/2013/12/31/17
FWIW, this issue you reported is related, but different from the one
I've described. The one I've described is based on sending ICMPv6
PTB<1280. RFC2460 states that when you receive an ICMPv6 PTB<1280 you
should add a Fragment Header to all packets sent to that destination
(i.e., produce the so called "IPv6 atomic fragments").
These "atomic fragments" have an offset=0, and MF=0 -- i.e., they are
not really fragmented.
Hence the trivial way to mitigate this attack is to drop incoming ICMPv6
PTB1280 (or, at the very least, don't react to them by sending all
subsequent packets with a Fragment Header).
Thanks!
Best regards,
--
Fernando Gont
SI6 Networks
e-mail: fgont@si6networks.com
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492
^ permalink raw reply
* Re: Sending undersized ARP packets with VXLAN L3 interface
From: Martin Rusko @ 2014-08-27 21:00 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Stephen Hemminger, Cong Wang, netdev
In-Reply-To: <53FE3E5C.8020804@gmail.com>
On Wed, Aug 27, 2014 at 10:23 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
> On 08/27/2014 04:01 PM, Martin Rusko wrote:
>> On Wed, Aug 27, 2014 at 8:45 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>> On 08/27/2014 02:42 PM, Stephen Hemminger wrote:
>>>> On Wed, 27 Aug 2014 13:52:03 -0400
>>>> Vlad Yasevich <vyasevich@gmail.com> wrote:
>>>>
>>>>> On 08/27/2014 01:28 PM, Cong Wang wrote:
>>>>>> On Wed, Aug 27, 2014 at 10:06 AM, Martin Rusko <martin.rusko@gmail.com> wrote:
>>>>>>>
>>>>>>> I'm wondering, where is the proper place to fix this. Should
>>>>>>> arp_create() function allocate skb big enough to produce ethernet
>>>>>>> frame with at least minimum size? Or is it somewhere in NIC drivers
>>>>>>> where small packets are padded with zeros?
>>>>>>
>>>>>> Drivers do that, for example e1000:
>>>>>>
>>>>>> /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>>>>>> * packets may get corrupted during padding by HW.
>>>>>> * To WA this issue, pad all small packets manually.
>>>>>> */
>>>>>> if (skb->len < ETH_ZLEN) {
>>>>>> if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>>>> return NETDEV_TX_OK;
>>>>>> skb->len = ETH_ZLEN;
>>>>>> skb_set_tail_pointer(skb, ETH_ZLEN);
>>>>>> }
>>>>>
>>>>>
>>>>> I think vxlan needs something like this:
>>>>>
>>>>> From: Vladislav Yasevich <vyasevich@gmail.com>
>>>>> Date: Wed, 27 Aug 2014 13:39:32 -0400
>>>>> Subject: [PATCH] vxlan: Pad short ethernet frames.
>>>>>
>>>>> If sending short ethernet frames from the vxlan device, pad
>>>>> them to minimum size so they can be forwarded after decapsulation.
>>>>>
>>>>> Reported-by: Martin Rusko <martin.rusko@gmail.com>
>>>>> Signed-off-by: Vladislav Yasevich <vyasevich@gmail.com>
>>>>> ---
>>>>> drivers/net/vxlan.c | 8 ++++++++
>>>>> 1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>>> index 1fb7b37..48267d4 100644
>>>>> --- a/drivers/net/vxlan.c
>>>>> +++ b/drivers/net/vxlan.c
>>>>> @@ -1939,6 +1939,14 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct
>>>>> net_device *dev)
>>>>> #endif
>>>>> }
>>>>>
>>>>> + /* Pad short frames so they can be forwarded after decapsulation */
>>>>> + if (skb->len < ETH_ZLEN) {
>>>>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>>> + return NETDEV_TX_OK;
>>>>> + skb->len = ETH_ZLEN;
>>>>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>>>>> + }
>>>>> +
>>>>> f = vxlan_find_mac(vxlan, eth->h_dest);
>>>>> did_rsc = false;
>>>>>
>>>>
>>>> No. The short frame is perfectly valid, over the VXLAN.
>>>> The system doing the decap and forwarding should be where any padding is added if necessary.
>>>>
>>
>> Well, RFC 7348 is not dealing with padding at all. Both deployment
>> scenarios listed in RFC, as well as most of the existing real life
>> deployments today (in my opinion) use VXLAN for bridged traffic. In
>> other words, frame encapsulated by VTEP is received first over some
>> ethernet interface (physical or virtual) which implies that the frame
>> is at least 64 bytes long already.
>>
>> Perhaps we're going to see more VXLAN interfaces in L3 mode, yet it
>> might be safer not to count on receiving VTEP doing the right thing
>> (pad small packets with zeros).
>>
>>>
>>> If that's the case, then Martin is most likely seeing a HW bug on the switch.
>>> I wonder how common such a bug might be?
>>>
>>> -vlad
>>>
>>
>> I see this on Vmware distributed virtual switch. Perhaps soon I will
>> be able to test it against HP 5930 switch. I'm going to try how Linux
>> bridge copes with it, now.
>
> Linux bridge will do just fine as it will pass the frame off to the hw driver
> which should pad things appropriately.
>
> -vlad
>
I can confirm that, now. After using namespaces to setup following topology:
[main host] ~~~~~ [switch ns] ------ [host ns]
~~~ = vxlan (on top of veth link)
---- = veth link
# namespace for the bridge with VTEP
ip netns add switch
# namespace for the remote host behind the bridge
ip netns add host
ip li add name veth0 type veth peer name veth1
ip li set veth1 netns switch
ip li set veth2 netns switch
ip li set veth3 netns host
ip ad add 192.0.2.1/30 brd + dev veth0
ip li set veth0 up
ip netns exec switch ip ad add 192.0.2.2/30 brd + dev veth1
ip netns exec switch ip li set veth1 up
ip li add name vxln0 type vxlan id 100 group 239.0.2.0 \
local 192.0.2.1 dev veth0 dstport 0
ip ad add 198.51.100.1/24 brd + dev vxln0
ip li set vxln0 up
ip netns exec switch ip li add name vxln1 type vxlan id 100 \
group 239.0.2.0 local 192.0.2.2 dev veth1 dstport 0
ip netns exec switch ip li add name vbr0 type bridge
ip netns exec switch ip li set vxln1 master vbr0
ip netns exec switch ip li set veth2 master vbr0
ip netns exec switch ip li set vxln1 up
ip netns exec switch ip li set veth2 up
ip netns exec switch ip li set vbr0 up
ip netns exec host ip ad add 198.51.100.2/24 brd + dev veth3
ip netns exec host ip li set veth3 up
I was able to arping remote host from the main host and when I tapped
to veth0 and veth2 interfaces, I could see small packets being
exchange without any issues.
Vlad, I'm going to recompile 3.16.1 kernel with your patch.
Regards,
Martin
^ permalink raw reply
* Re: [RFC PATCH net-next] ipv6: stop sending PTB packets for MTU < 1280
From: Hagen Paul Pfeifer @ 2014-08-27 20:57 UTC (permalink / raw)
To: Fernando Gont; +Cc: Hannes Frederic Sowa, netdev
In-Reply-To: <53FE4086.8040708@si6networks.com>
On 27 August 2014 22:33, Fernando Gont <fgont@si6networks.com> wrote:
> Hence the trivial way to mitigate this attack is to drop incoming ICMPv6
> PTB1280 (or, at the very least, don't react to them by sending all
> subsequent packets with a Fragment Header).
Any news from the v6ops people - what is your suggestion Fernando?
Should we wait for more v6ops input?
Hagen
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:53 UTC (permalink / raw)
To: therbert
Cc: cwang, netdev, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty,
dborkman, brouer
In-Reply-To: <CA+mtBx-zqi0mzMynZrxfcfz7koLH+B4+kD6o9+DnS7N1ww16Sw@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 27 Aug 2014 12:31:15 -0700
> On Wed, Aug 27, 2014 at 11:28 AM, Cong Wang <cwang@twopensource.com> wrote:
>> On Mon, Aug 25, 2014 at 4:34 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> Given Jesper's performance numbers, it's not the way to go.
>>>
>>> Instead, go with a signalling scheme via new boolean skb->xmit_more.
>>>
>>> This has several advantages:
>>>
>>> 1) Nearly trivial driver support, just protect the tail pointer
>>> update with the skb->xmit_more check.
>>>
>>> 2) No extra indirect calls in the non-deferral cases.
>>>
>>
>> First of all, I missed your discussion at kernel summit.
>>
>> Second of all, I am not familiar with hardware NIC drivers.
>>
>> But for me, it looks like you are trying to pend some more packets
>> in a TX queue until the driver decides to flush them all in one shot.
>> So if that is true, doesn't this mean the latency of first packet pending
>> in this queue will increase and network traffic will be more bursty for
>> the receiver??
>>
> I suspect this won't be an big issue. The dequeue is still work
> conserving and BQL limit already ensures that HW queue doesn't drain
> completely when packets are pending in the qdisc-- I doubt this will
> increase BQL limits, but that should be verified. We might see some
> latency increase for a batch sent on an idle link (possible with
> GSO)-- if this is a concern we could arrange flush on sending packets
> on idle links.
That's also correct.
The issue to handle specially is the initial send on a TX queue which
is empty or close to being empty.
Probably we want some kind of exponential backoff type scheme, so
assuming we have an empty TX queue we'd trigger TX on the first
packet, then the third, then the 7th. Assuming we had that many to
send at once.
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:51 UTC (permalink / raw)
To: cwang
Cc: netdev, therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty,
dborkman, brouer
In-Reply-To: <CAHA+R7MJX3Du8ggOo2GqGA4rXvY30YOpCFqS4PBN2ZNE=kwKuA@mail.gmail.com>
From: Cong Wang <cwang@twopensource.com>
Date: Wed, 27 Aug 2014 11:28:25 -0700
> But for me, it looks like you are trying to pend some more packets
> in a TX queue until the driver decides to flush them all in one shot.
> So if that is true, doesn't this mean the latency of first packet pending
> in this queue will increase and network traffic will be more bursty for
> the receiver??
We intend to turn this so that it doesn't introduce latency.
The situation where we have the largest opportunity to perform
this batching is when the device queue has been stopped and is
started back up.
Usually at this moment the TX queue is %75 full and we have room
now for a couple of packets.
Therefore, delaying the triggering of the TX for this new set of
packets will have no effect on latency because the device is still
busy transmitting the rest of the TX queue.
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:48 UTC (permalink / raw)
To: brouer
Cc: eric.dumazet, hannes, netdev, therbert, jhs, edumazet,
jeffrey.t.kirsher, rusty, dborkman
In-Reply-To: <20140827155651.04fe49d0@redhat.com>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Wed, 27 Aug 2014 15:56:51 +0200
> I do worry a bit, if writing skb->xmit_more in a more cache cold
> scenarios could hurt us (as that cacheline seems to be read mostly),
> but this is not a problem right now because it will always be cache
> hot, as we always clear the entire SKB in alloc.
And as I mentioned to Eric, xmit_more sits right next to the queue
number which is also dirtied in the TX path.
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:46 UTC (permalink / raw)
To: eric.dumazet
Cc: hannes, netdev, therbert, jhs, edumazet, jeffrey.t.kirsher, rusty,
dborkman, brouer
In-Reply-To: <1409145838.3173.41.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 27 Aug 2014 06:23:58 -0700
> On Wed, 2014-08-27 at 14:31 +0200, Hannes Frederic Sowa wrote:
>
>> One thing one should keep in mind is, that there must be a skb available
>> to trigger the flush, maybe this will hurt us one day.
>>
>> Thinking more about it should we go with a coccinelle script and
>> replace/extend ndo_start_xmit with an additional argument?
>
> This will be a pain for backports and things like that.
Agreed.
> skb->xmit_more is a bit annoying, because it consumes one bit in all
> skbs, while it could be one byte per cpu as ndo_start_xmit() is called
> while BH are disabled.
>
> It also forces a cache line dirtying, that will hurt qdisc like HTB
> where skbs can be cooked/enqueued by remote cpus.
The xmit_more value sits in the same cache line as the queue number,
which also gets dirtied in the TX fast path.
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:45 UTC (permalink / raw)
To: hannes
Cc: netdev, therbert, jhs, edumazet, jeffrey.t.kirsher, rusty,
dborkman, brouer
In-Reply-To: <1409142672.26515.24.camel@localhost>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 27 Aug 2014 14:31:12 +0200
> On Mo, 2014-08-25 at 16:34 -0700, David Miller wrote:
>> Given Jesper's performance numbers, it's not the way to go.
>>
>> Instead, go with a signalling scheme via new boolean skb->xmit_more.
>>
>> This has several advantages:
>>
>> 1) Nearly trivial driver support, just protect the tail pointer
>> update with the skb->xmit_more check.
>
> One thing one should keep in mind is, that there must be a skb available
> to trigger the flush, maybe this will hurt us one day.
>
> Thinking more about it should we go with a coccinelle script and
> replace/extend ndo_start_xmit with an additional argument?
>
> We can also add a new function pointer and call that instead of
> ndo_start_xmit. I think only the callq *%rax hurts performance.
I don't think we will have any problems here, the caller will always
be the entity which analyzes the upcoming set of SKBs to submit and
tag them properly.
I really do not want to add a new OP and I even more so do not want to
adjust the ndo_start_xmit() signature. It's effect is far reaching,
and for absolutely no gain as far as I can see.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/2] Get rid of ndo_xmit_flush
From: David Miller @ 2014-08-27 20:43 UTC (permalink / raw)
To: brouer
Cc: netdev, therbert, jhs, hannes, edumazet, jeffrey.t.kirsher, rusty,
dborkman
In-Reply-To: <20140827141918.198cba38@redhat.com>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Wed, 27 Aug 2014 14:19:18 +0200
> On Mon, 25 Aug 2014 16:34:58 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
>
>> Given Jesper's performance numbers, it's not the way to go.
>>
>> Instead, go with a signalling scheme via new boolean skb->xmit_more.
>>
>> This has several advantages:
>>
>> 1) Nearly trivial driver support, just protect the tail pointer
>> update with the skb->xmit_more check.
>>
>> 2) No extra indirect calls in the non-deferral cases.
>
> Even-though it is obvious that this new API skb->xmit_more will not
> hurt performance, especially given skb->xmit_more is always 0 in this
> kernel, I've still run my pktgen performance tests.
>
> Compared to baseline[1]: (averaged 5609929 pps) (details below signature)
> * (1/5609929*10^9)-(1/5603728*10^9) = -0.197ns
>
> As expected, this API does not hurt performance (as -0.197ns is below
> our accuracy levels).
>
> [1] http://thread.gmane.org/gmane.linux.network/327254/focus=327838
Thanks for validating Jesper :-)
^ permalink raw reply
* [PATCH v8 net-next 1/2] net: filter: add "load 64-bit immediate" eBPF instruction
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1409171833-6979-1-git-send-email-ast@plumgrid.com>
add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.
x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn
Note that old eBPF programs are binary compatible with new interpreter.
It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)
User space generated programs will use this instruction to load constants only.
To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop pseudo marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on single generic BPF_LD_IMM64.
User space never operates on direct pointers and verifier can easily
recognize request_for_pointer_pseudo_insn vs other instructions.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Documentation/networking/filter.txt | 8 +++++++-
arch/x86/net/bpf_jit_comp.c | 17 +++++++++++++++++
include/linux/filter.h | 18 ++++++++++++++++++
kernel/bpf/core.c | 5 +++++
lib/test_bpf.c | 21 +++++++++++++++++++++
5 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -951,7 +951,7 @@ Size modifier is one of ...
Mode modifier is one of:
- BPF_IMM 0x00 /* classic BPF only, reserved in eBPF */
+ BPF_IMM 0x00 /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
BPF_ABS 0x20
BPF_IND 0x40
BPF_MEM 0x60
@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
2 byte atomic increments are not supported.
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
Testing
-------
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c59530..98837147ee57 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
break;
+ case BPF_LD | BPF_IMM | BPF_DW:
+ if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+ insn[1].dst_reg != 0 || insn[1].off != 0) {
+ /* verifier must catch invalid insns */
+ pr_err("invalid BPF_LD_IMM64 insn\n");
+ return -EINVAL;
+ }
+
+ /* movabsq %rax, imm64 */
+ EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+ EMIT(insn[0].imm, 4);
+ EMIT(insn[1].imm, 4);
+
+ insn++;
+ i++;
+ break;
+
/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
case BPF_ALU | BPF_MOD | BPF_X:
case BPF_ALU | BPF_DIV | BPF_X:
diff --git a/include/linux/filter.h b/include/linux/filter.h
index a5227ab8ccb1..f3262b598262 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -161,6 +161,24 @@ enum {
.off = 0, \
.imm = IMM })
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7f0dbcbb34af..0434c2170f2b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -180,6 +180,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+ [BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
};
void *ptr;
int off;
@@ -239,6 +240,10 @@ select_insn:
ALU64_MOV_K:
DST = IMM;
CONT;
+ LD_IMM_DW:
+ DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+ insn++;
+ CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
CONT;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 8c66c6aace04..46ab1a7ef135 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
{ },
{ { 1, 0 } },
},
+ {
+ "load 64-bit immediate",
+ .u.insns_int = {
+ BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_MOV64_REG(R2, R1),
+ BPF_MOV64_REG(R3, R2),
+ BPF_ALU64_IMM(BPF_RSH, R2, 32),
+ BPF_ALU64_IMM(BPF_LSH, R3, 32),
+ BPF_ALU64_IMM(BPF_RSH, R3, 32),
+ BPF_ALU64_IMM(BPF_MOV, R0, 0),
+ BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+ BPF_EXIT_INSN(),
+ BPF_ALU64_IMM(BPF_MOV, R0, 1),
+ BPF_EXIT_INSN(),
+ },
+ INTERNAL,
+ { },
+ { { 0, 1 } }
+ },
};
static struct net_device dev;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v8 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409171833-6979-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
allow user space to generate eBPF programs
uapi/linux/bpf.h: eBPF instruction set definition
linux/filter.h: the rest
This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.
These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.
Full eBPF ISA description is in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
include/linux/filter.h | 56 +-------------------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f3262b598262..3150666cd4b9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -9,53 +9,7 @@
#include <linux/skbuff.h>
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
-
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64 0x07 /* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW 0x18 /* double word */
-#define BPF_XADD 0xc0 /* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV 0xb0 /* mov reg to reg */
-#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END 0xd0 /* flags for endianness conversion: */
-#define BPF_TO_LE 0x00 /* convert to little-endian */
-#define BPF_TO_BE 0x08 /* convert to big-endian */
-#define BPF_FROM_LE BPF_TO_LE
-#define BPF_FROM_BE BPF_TO_BE
-
-#define BPF_JNE 0x50 /* jump != */
-#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
-#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
-#define BPF_CALL 0x80 /* function call */
-#define BPF_EXIT 0x90 /* function return */
-
-/* Register numbers */
-enum {
- BPF_REG_0 = 0,
- BPF_REG_1,
- BPF_REG_2,
- BPF_REG_3,
- BPF_REG_4,
- BPF_REG_5,
- BPF_REG_6,
- BPF_REG_7,
- BPF_REG_8,
- BPF_REG_9,
- BPF_REG_10,
- __MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG __MAX_BPF_REG
+#include <uapi/linux/bpf.h>
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
@@ -317,14 +271,6 @@ enum {
#define SK_RUN_FILTER(filter, ctx) \
(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
-struct bpf_insn {
- __u8 code; /* opcode */
- __u8 dst_reg:4; /* dest register */
- __u8 src_reg:4; /* source register */
- __s16 off; /* signed offset */
- __s32 imm; /* signed immediate constant */
-};
-
#ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */
struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
+header-y += bpf.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * 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.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64 0x07 /* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW 0x18 /* double word */
+#define BPF_XADD 0xc0 /* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV 0xb0 /* mov reg to reg */
+#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END 0xd0 /* flags for endianness conversion: */
+#define BPF_TO_LE 0x00 /* convert to little-endian */
+#define BPF_TO_BE 0x08 /* convert to big-endian */
+#define BPF_FROM_LE BPF_TO_LE
+#define BPF_FROM_BE BPF_TO_BE
+
+#define BPF_JNE 0x50 /* jump != */
+#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
+#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
+#define BPF_CALL 0x80 /* function call */
+#define BPF_EXIT 0x90 /* function return */
+
+/* Register numbers */
+enum {
+ BPF_REG_0 = 0,
+ BPF_REG_1,
+ BPF_REG_2,
+ BPF_REG_3,
+ BPF_REG_4,
+ BPF_REG_5,
+ BPF_REG_6,
+ BPF_REG_7,
+ BPF_REG_8,
+ BPF_REG_9,
+ BPF_REG_10,
+ __MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG __MAX_BPF_REG
+
+struct bpf_insn {
+ __u8 code; /* opcode */
+ __u8 dst_reg:4; /* dest register */
+ __u8 src_reg:4; /* source register */
+ __s16 off; /* signed offset */
+ __s32 imm; /* signed immediate constant */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v8 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-08-27 20:37 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Brendan Gregg, Namhyung Kim, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
Hi David,
I've been thinking on the minimum first patch set.
Came up with the following two:
1st patch adds 'load 64-bit immediate' instruction which by itself
is harmless and used to load constants only. In the future we may
add pseudo variants of this insn, so user space can request
internal kernel pointer. More detailed explanation in the commit log.
2nd patch exposed eBPF ISA to user space. It moves 55 lines from
filter.h into uapi/linux/bpf.h
Though there is no way currently to load eBPF programs from user
space, this patch shows the intent that eventually it will be possible.
The main goal here is to unblock LLVM upstreaming process.
Once these two are in, I can start posting LLVM RFCs to llvmdev list
and getting compiler bits in, so by the time bpf syscall and verifier
are in, we may have LLVM backend upstreamed as well.
LLVM wouldn't care what eBPF is used for, whether syscall is used
or some other mechanism. It just compiles C into eBPF ISA.
So these two patches are sufficient to start LLVM upstreaming.
All,
why do we need all of these?
Same reason why we're still using classic BPF and keep trying to extend it.
There are places in kernel where safe dynamic programs are mandatory.
network traffic capture needs in-kernel filtering,
seccomp needs safe mini programs to sandbox applications,
tracing needs them to filter events and so on.
Few LWN articles that explain things way better than my commit logs:
http://lwn.net/Articles/599755/
http://lwn.net/Articles/603983/
http://lwn.net/Articles/606089/
http://lwn.net/Articles/575531/
The first target for eBPF is to have dtrace equivalent that can be
used in _production_. Safety of programs is paramount.
Just like performance. eBPF programs in tracing should not affect
performance of production severs, so huge effort on optimizing last bit.
ebpf+tracing, ebpf+seccomp, ebpf+sockets are the most obvious use cases.
ebpf+ovs is the one we use in large kvm hypervisors.
40Gbps of traffic are going through these programs, so performance
and safety are vital. Performance implications ruling out run-time checks
in critical path, so verifier is large mainly because it needs to do
all the checks during static analysis.
I think next patch set will include syscall shell with minimal functionality,
syscall doc and simple test.
Alexei Starovoitov (2):
net: filter: add "load 64-bit immediate" eBPF instruction
net: filter: split filter.h and expose eBPF to user space
Documentation/networking/filter.txt | 8 +++-
arch/x86/net/bpf_jit_comp.c | 17 ++++++++
include/linux/filter.h | 74 +++++++++--------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 ++++++++++++++++++++++++++++++
kernel/bpf/core.c | 5 +++
lib/test_bpf.c | 21 ++++++++++
7 files changed, 135 insertions(+), 56 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
--
1.7.9.5
^ permalink raw reply
* Re: Sending undersized ARP packets with VXLAN L3 interface
From: Vlad Yasevich @ 2014-08-27 20:23 UTC (permalink / raw)
To: Martin Rusko; +Cc: Stephen Hemminger, Cong Wang, netdev
In-Reply-To: <CAMYYbY4KHTkkv6peT14S8mCzcQJ8wROnHWj5tMy563H-w79KMg@mail.gmail.com>
On 08/27/2014 04:01 PM, Martin Rusko wrote:
> On Wed, Aug 27, 2014 at 8:45 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
>> On 08/27/2014 02:42 PM, Stephen Hemminger wrote:
>>> On Wed, 27 Aug 2014 13:52:03 -0400
>>> Vlad Yasevich <vyasevich@gmail.com> wrote:
>>>
>>>> On 08/27/2014 01:28 PM, Cong Wang wrote:
>>>>> On Wed, Aug 27, 2014 at 10:06 AM, Martin Rusko <martin.rusko@gmail.com> wrote:
>>>>>>
>>>>>> I'm wondering, where is the proper place to fix this. Should
>>>>>> arp_create() function allocate skb big enough to produce ethernet
>>>>>> frame with at least minimum size? Or is it somewhere in NIC drivers
>>>>>> where small packets are padded with zeros?
>>>>>
>>>>> Drivers do that, for example e1000:
>>>>>
>>>>> /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>>>>> * packets may get corrupted during padding by HW.
>>>>> * To WA this issue, pad all small packets manually.
>>>>> */
>>>>> if (skb->len < ETH_ZLEN) {
>>>>> if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>>> return NETDEV_TX_OK;
>>>>> skb->len = ETH_ZLEN;
>>>>> skb_set_tail_pointer(skb, ETH_ZLEN);
>>>>> }
>>>>
>>>>
>>>> I think vxlan needs something like this:
>>>>
>>>> From: Vladislav Yasevich <vyasevich@gmail.com>
>>>> Date: Wed, 27 Aug 2014 13:39:32 -0400
>>>> Subject: [PATCH] vxlan: Pad short ethernet frames.
>>>>
>>>> If sending short ethernet frames from the vxlan device, pad
>>>> them to minimum size so they can be forwarded after decapsulation.
>>>>
>>>> Reported-by: Martin Rusko <martin.rusko@gmail.com>
>>>> Signed-off-by: Vladislav Yasevich <vyasevich@gmail.com>
>>>> ---
>>>> drivers/net/vxlan.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>>> index 1fb7b37..48267d4 100644
>>>> --- a/drivers/net/vxlan.c
>>>> +++ b/drivers/net/vxlan.c
>>>> @@ -1939,6 +1939,14 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct
>>>> net_device *dev)
>>>> #endif
>>>> }
>>>>
>>>> + /* Pad short frames so they can be forwarded after decapsulation */
>>>> + if (skb->len < ETH_ZLEN) {
>>>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>> + return NETDEV_TX_OK;
>>>> + skb->len = ETH_ZLEN;
>>>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>>>> + }
>>>> +
>>>> f = vxlan_find_mac(vxlan, eth->h_dest);
>>>> did_rsc = false;
>>>>
>>>
>>> No. The short frame is perfectly valid, over the VXLAN.
>>> The system doing the decap and forwarding should be where any padding is added if necessary.
>>>
>
> Well, RFC 7348 is not dealing with padding at all. Both deployment
> scenarios listed in RFC, as well as most of the existing real life
> deployments today (in my opinion) use VXLAN for bridged traffic. In
> other words, frame encapsulated by VTEP is received first over some
> ethernet interface (physical or virtual) which implies that the frame
> is at least 64 bytes long already.
>
> Perhaps we're going to see more VXLAN interfaces in L3 mode, yet it
> might be safer not to count on receiving VTEP doing the right thing
> (pad small packets with zeros).
>
>>
>> If that's the case, then Martin is most likely seeing a HW bug on the switch.
>> I wonder how common such a bug might be?
>>
>> -vlad
>>
>
> I see this on Vmware distributed virtual switch. Perhaps soon I will
> be able to test it against HP 5930 switch. I'm going to try how Linux
> bridge copes with it, now.
Linux bridge will do just fine as it will pass the frame off to the hw driver
which should pad things appropriately.
-vlad
>
> Many thanks for the patch anyway!
>
> Regards,
> Martin
>
^ permalink raw reply
* Re: [PATCH v2] ethernet: arc: Add support for Rockchip SoC layer device tree bindings
From: Heiko Stübner @ 2014-08-27 20:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Romain Perier, linux-rockchip, linux-arm-kernel, netdev
In-Reply-To: <3619843.a6HHkMsuOu@wuerfel>
Am Mittwoch, 27. August 2014, 22:11:29 schrieb Arnd Bergmann:
> On Wednesday 27 August 2014 06:55:27 Romain Perier wrote:
> > This patch defines a platform glue layer for Rockchip SoCs which
> > support arc-emac driver. It ensures that regulator for the rmii is on
> > before trying to connect to the ethernet controller. It applies right
> > speed and mode changes to the grf when ethernet settings change.
> >
> > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > ---
> >
> > drivers/net/ethernet/arc/Kconfig | 15 +++
> > drivers/net/ethernet/arc/Makefile | 1 +
> > drivers/net/ethernet/arc/emac.h | 2 +
> > drivers/net/ethernet/arc/emac_main.c | 2 +
> > drivers/net/ethernet/arc/emac_rockchip.c | 224
> > +++++++++++++++++++++++++++++++ 5 files changed, 244 insertions(+)
> > create mode 100644 drivers/net/ethernet/arc/emac_rockchip.c
> >
> > diff --git a/drivers/net/ethernet/arc/Kconfig
> > b/drivers/net/ethernet/arc/Kconfig index 89e04fd..6d96a82 100644
> > --- a/drivers/net/ethernet/arc/Kconfig
> > +++ b/drivers/net/ethernet/arc/Kconfig
> > @@ -32,4 +32,19 @@ config ARC_EMAC
> >
> > non-standard on-chip ethernet device ARC EMAC 10/100 is used.
> > Say Y here if you have such a board. If unsure, say N.
> >
> > +config EMAC_ROCKCHIP
> > + tristate "Rockchip EMAC support"
> > + select ARC_EMAC_CORE
> > + depends on OF_IRQ
> > + depends on OF_NET
> > + depends on ARCH_ROCKCHIP
> > + depends on REGULATOR_ACT8865
> > + depends on SMSC_PHY
> > + depends on MFD_SYSCON
>
> You should generally not add 'depends on' for specific drivers out
> of a subsystems. Just list the build-time dependencies, like
>
> depends on OF_IRQ && OF_NET && PHYLIB && REGULATOR && MFD_SYSCON
in this case even MFD_SYSCON can go away, as the syscon functions provide
stubs for !MFD_SYSCON .
[...]
> > + rate = 50000000;
>
> Where does this number come from?
The RMII interface needs always a rate of 50MHz ... my questions would be why
the 50000000 are not in the clk_set_rate directly and need an extra var :-)
Heiko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox