* [RFC PATCH 2/3] igb: use new eth_get_headlen interface
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
In-Reply-To: <20140904230958.12376.2845.stgit@ahduyck-bv4.jf.intel.com>
Update igb to drop the igb_get_headlen function in favor of eth_get_headlen.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 109 -----------------------------
1 file changed, 1 insertion(+), 108 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 89de7fe..4c023f0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6769,113 +6769,6 @@ static bool igb_is_non_eop(struct igb_ring *rx_ring,
}
/**
- * igb_get_headlen - determine size of header for LRO/GRO
- * @data: pointer to the start of the headers
- * @max_len: total length of section to find headers in
- *
- * This function is meant to determine the length of headers that will
- * be recognized by hardware for LRO, and GRO offloads. The main
- * motivation of doing this is to only perform one pull for IPv4 TCP
- * packets so that we can do basic things like calculating the gso_size
- * based on the average data per packet.
- **/
-static unsigned int igb_get_headlen(unsigned char *data,
- unsigned int max_len)
-{
- union {
- unsigned char *network;
- /* l2 headers */
- struct ethhdr *eth;
- struct vlan_hdr *vlan;
- /* l3 headers */
- struct iphdr *ipv4;
- struct ipv6hdr *ipv6;
- } hdr;
- __be16 protocol;
- u8 nexthdr = 0; /* default to not TCP */
- u8 hlen;
-
- /* this should never happen, but better safe than sorry */
- if (max_len < ETH_HLEN)
- return max_len;
-
- /* initialize network frame pointer */
- hdr.network = data;
-
- /* set first protocol and move network header forward */
- protocol = hdr.eth->h_proto;
- hdr.network += ETH_HLEN;
-
- /* handle any vlan tag if present */
- if (protocol == htons(ETH_P_8021Q)) {
- if ((hdr.network - data) > (max_len - VLAN_HLEN))
- return max_len;
-
- protocol = hdr.vlan->h_vlan_encapsulated_proto;
- hdr.network += VLAN_HLEN;
- }
-
- /* handle L3 protocols */
- if (protocol == htons(ETH_P_IP)) {
- if ((hdr.network - data) > (max_len - sizeof(struct iphdr)))
- return max_len;
-
- /* access ihl as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[0] & 0x0F) << 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct iphdr))
- return hdr.network - data;
-
- /* record next protocol if header is present */
- if (!(hdr.ipv4->frag_off & htons(IP_OFFSET)))
- nexthdr = hdr.ipv4->protocol;
- } else if (protocol == htons(ETH_P_IPV6)) {
- if ((hdr.network - data) > (max_len - sizeof(struct ipv6hdr)))
- return max_len;
-
- /* record next protocol */
- nexthdr = hdr.ipv6->nexthdr;
- hlen = sizeof(struct ipv6hdr);
- } else {
- return hdr.network - data;
- }
-
- /* relocate pointer to start of L4 header */
- hdr.network += hlen;
-
- /* finally sort out TCP */
- if (nexthdr == IPPROTO_TCP) {
- if ((hdr.network - data) > (max_len - sizeof(struct tcphdr)))
- return max_len;
-
- /* access doff as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[12] & 0xF0) >> 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct tcphdr))
- return hdr.network - data;
-
- hdr.network += hlen;
- } else if (nexthdr == IPPROTO_UDP) {
- if ((hdr.network - data) > (max_len - sizeof(struct udphdr)))
- return max_len;
-
- hdr.network += sizeof(struct udphdr);
- }
-
- /* If everything has gone correctly hdr.network should be the
- * data section of the packet and will be the end of the header.
- * If not then it probably represents the end of the last recognized
- * header.
- */
- if ((hdr.network - data) < max_len)
- return hdr.network - data;
- else
- return max_len;
-}
-
-/**
* igb_pull_tail - igb specific version of skb_pull_tail
* @rx_ring: rx descriptor ring packet is being transacted on
* @rx_desc: pointer to the EOP Rx descriptor
@@ -6919,7 +6812,7 @@ static void igb_pull_tail(struct igb_ring *rx_ring,
/* we need the header to contain the greater of either ETH_HLEN or
* 60 bytes if the skb->len is less than 60 for skb_pad.
*/
- pull_len = igb_get_headlen(va, IGB_RX_HDR_LEN);
+ pull_len = eth_get_headlen(va, IGB_RX_HDR_LEN);
/* align pull length to size of long to optimize memcpy performance */
skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
^ permalink raw reply related
* [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
This series replaced igb_get_headlen and ixgbe_get_headlen with a generic
function named eth_get_headlen.
This is a first pass at this and has been compile tested only. I'm mainly
looking for feedback on any issues with the approach I have taken or any
complains about the eth_get_headlen function or the changes to __skb_get_poff
in the first patch.
I'll try running this through some stress/performance testing tomorrow to
see how it compares to the original functions.
---
Alexander Duyck (3):
net: Add function for parsing the header length out of linear ethernet frames
igb: use new eth_get_headlen interface
ixgbe: use new eth_get_headlen interface
drivers/net/ethernet/intel/igb/igb_main.c | 109 -----------------------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 116 -------------------------
include/linux/etherdevice.h | 1
include/linux/skbuff.h | 2
include/net/flow_keys.h | 2
net/core/flow_dissector.c | 41 ++++++---
net/ethernet/eth.c | 27 ++++++
7 files changed, 60 insertions(+), 238 deletions(-)
--
^ permalink raw reply
* Re: Regression: TCP connections fail over wireless: bad cksum?
From: Ted Percival @ 2014-09-04 23:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1409871007.26422.125.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/04/2014 04:50 PM, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 14:11 -0600, Ted Percival wrote:
>> Yesterday's linux-next build introduced a problem with wireless
>> networking on my machine. ie. next-20140901 worked fine but
>> next-20140902 does not seem able to sustain a TCP connection over
>> wireless. Wired networking works fine. I am using the brcmsmac driver
>> and the hardware is "Broadcom Corporation BCM4313 802.11b/g/n Wireless
>> LAN Controller (rev 01)".
>>
>> Pings, even large pings (ping -s 16000) work fine but TCP connections hang.
>>
>> I looked through the changes between the bad & good commits from the net
>> & net-next trees and I wonder if some of the changes to checksumming
>> have surfaced a problem with this driver. When I look at a tcpdump, it
>> indicates that all the checksums are wrong (although I don't know if
>> that is just due to hardware offload).
>>
>> Here is a short trace of the hung connection attempt of
>> curl http://lwn.net/
>>
>> $ sudo tcpdump -vvn -i wlan0 port 80
>> tcpdump: listening on wlan0, link-type EN10MB (Ethernet), capture size
>> 65535 bytes
>> 13:20:15.120770 IP (tos 0x0, ttl 64, id 22734, offset 0, flags [DF],
>> proto TCP (6), length 60)
>> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
>> (incorrect -> 0x4fac), seq 2861986597, win 29200, options [mss
>> 1460,sackOK,TS val 142315 ecr 0,nop,wscale 7], length 0
>> 13:20:16.121755 IP (tos 0x0, ttl 64, id 22735, offset 0, flags [DF],
>> proto TCP (6), length 60)
>> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
>> (incorrect -> 0x4bc3), seq 2861986597, win 29200, options [mss
>> 1460,sackOK,TS val 143316 ecr 0,nop,wscale 7], length 0
>> 13:20:18.125748 IP (tos 0x0, ttl 64, id 22736, offset 0, flags [DF],
>> proto TCP (6), length 60)
>> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
>> (incorrect -> 0x43ef), seq 2861986597, win 29200, options [mss
>> 1460,sackOK,TS val 145320 ecr 0,nop,wscale 7], length 0
>> 13:20:22.133743 IP (tos 0x0, ttl 64, id 22737, offset 0, flags [DF],
>> proto TCP (6), length 60)
>> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
>> (incorrect -> 0x3447), seq 2861986597, win 29200, options [mss
>> 1460,sackOK,TS val 149328 ecr 0,nop,wscale 7], length 0
>> 13:20:30.149754 IP (tos 0x0, ttl 64, id 22738, offset 0, flags [DF],
>> proto TCP (6), length 60)
>> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
>> (incorrect -> 0x14f7), seq 2861986597, win 29200, options [mss
>> 1460,sackOK,TS val 157344 ecr 0,nop,wscale 7], length 0
>>
>>
>> I don't see anything that looks related in dmesg. The only brcmsmac
>> messages I see are:
>>
>> [ 567.122218] brcmsmac bcma0:0: brcmsmac: brcms_ops_bss_info_changed:
>> associated
>> [ 567.122226] brcmsmac bcma0:0: brcms_ops_bss_info_changed: arp
>> filtering: 1 addresses (implement)
>> [ 567.122231] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
>> enabled: true (implement)
>> [ 567.192283] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
>> enabled: true (implement)
>>
>> I am writing to linux-netdev rather than linux-wireless because
>> according to Next/SHA1s the wireless & wireless-next trees were not
>> updated between next-20140901 and next-20140902, but the net & net-next
>> trees were updated, so maybe the regression came from there. (I haven't
>> tested next-20140903 because it won't boot for unrelated reasons.)
>>
>> Let me know if I should just file this in Bugzilla or what information I
>> can provide to help track this down, if it hasn't already been identified.
>>
>> The here are the good (-) and bad (+) trees from Next/SHA1s at the
>> next-* tags mentioned earlier that I built from.
>>
>> -net 38ab1fa981d543e1b00f4ffbce4ddb480cd2effe
>> +net cc25f0cbe4409d6a573b1f3bf7020d5b04076ee9
>>
>> -net-next dace1b54726bffe1c009f7661e3cee6b762f30c8
>> +net-next 364a9e93243d1785f310c0964af0e24bf1adac03
>>
>
> Could you post
>
> ethtool -k wlan0
>
> And try
>
> ethtool -K wlan tx off
# ethtool -k wlan0
Features for wlan0:
rx-checksumming: off [fixed]
tx-checksumming: off
tx-checksum-ipv4: off [fixed]
tx-checksum-ip-generic: off [fixed]
tx-checksum-ipv6: off [fixed]
tx-checksum-fcoe-crc: off [fixed]
tx-checksum-sctp: off [fixed]
scatter-gather: off
tx-scatter-gather: off [fixed]
tx-scatter-gather-fraglist: off [fixed]
tcp-segmentation-offload: off
tx-tcp-segmentation: off [fixed]
tx-tcp-ecn-segmentation: off [fixed]
tx-tcp6-segmentation: off [fixed]
udp-fragmentation-offload: off [fixed]
generic-segmentation-offload: off [requested on]
generic-receive-offload: on
large-receive-offload: off [fixed]
rx-vlan-offload: off [fixed]
tx-vlan-offload: off [fixed]
ntuple-filters: off [fixed]
receive-hashing: off [fixed]
highdma: off [fixed]
rx-vlan-filter: off [fixed]
vlan-challenged: off [fixed]
tx-lockless: off [fixed]
netns-local: on [fixed]
tx-gso-robust: off [fixed]
tx-fcoe-segmentation: off [fixed]
tx-gre-segmentation: off [fixed]
tx-ipip-segmentation: off [fixed]
tx-sit-segmentation: off [fixed]
tx-udp_tnl-segmentation: off [fixed]
tx-mpls-segmentation: off [fixed]
fcoe-mtu: off [fixed]
tx-nocache-copy: off
loopback: off [fixed]
rx-fcs: off [fixed]
rx-all: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]
l2-fwd-offload: off [fixed]
busy-poll: off [fixed]
# ethtool -K wlan0 tx off
Cannot change tx-checksumming
I will try to isolate the commit that caused the regression.
^ permalink raw reply
* Re: Regression: TCP connections fail over wireless: bad cksum?
From: Eric Dumazet @ 2014-09-04 22:50 UTC (permalink / raw)
To: Ted Percival; +Cc: netdev
In-Reply-To: <5408C788.4050401@tedp.id.au>
On Thu, 2014-09-04 at 14:11 -0600, Ted Percival wrote:
> Yesterday's linux-next build introduced a problem with wireless
> networking on my machine. ie. next-20140901 worked fine but
> next-20140902 does not seem able to sustain a TCP connection over
> wireless. Wired networking works fine. I am using the brcmsmac driver
> and the hardware is "Broadcom Corporation BCM4313 802.11b/g/n Wireless
> LAN Controller (rev 01)".
>
> Pings, even large pings (ping -s 16000) work fine but TCP connections hang.
>
> I looked through the changes between the bad & good commits from the net
> & net-next trees and I wonder if some of the changes to checksumming
> have surfaced a problem with this driver. When I look at a tcpdump, it
> indicates that all the checksums are wrong (although I don't know if
> that is just due to hardware offload).
>
> Here is a short trace of the hung connection attempt of
> curl http://lwn.net/
>
> $ sudo tcpdump -vvn -i wlan0 port 80
> tcpdump: listening on wlan0, link-type EN10MB (Ethernet), capture size
> 65535 bytes
> 13:20:15.120770 IP (tos 0x0, ttl 64, id 22734, offset 0, flags [DF],
> proto TCP (6), length 60)
> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
> (incorrect -> 0x4fac), seq 2861986597, win 29200, options [mss
> 1460,sackOK,TS val 142315 ecr 0,nop,wscale 7], length 0
> 13:20:16.121755 IP (tos 0x0, ttl 64, id 22735, offset 0, flags [DF],
> proto TCP (6), length 60)
> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
> (incorrect -> 0x4bc3), seq 2861986597, win 29200, options [mss
> 1460,sackOK,TS val 143316 ecr 0,nop,wscale 7], length 0
> 13:20:18.125748 IP (tos 0x0, ttl 64, id 22736, offset 0, flags [DF],
> proto TCP (6), length 60)
> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
> (incorrect -> 0x43ef), seq 2861986597, win 29200, options [mss
> 1460,sackOK,TS val 145320 ecr 0,nop,wscale 7], length 0
> 13:20:22.133743 IP (tos 0x0, ttl 64, id 22737, offset 0, flags [DF],
> proto TCP (6), length 60)
> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
> (incorrect -> 0x3447), seq 2861986597, win 29200, options [mss
> 1460,sackOK,TS val 149328 ecr 0,nop,wscale 7], length 0
> 13:20:30.149754 IP (tos 0x0, ttl 64, id 22738, offset 0, flags [DF],
> proto TCP (6), length 60)
> 10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
> (incorrect -> 0x14f7), seq 2861986597, win 29200, options [mss
> 1460,sackOK,TS val 157344 ecr 0,nop,wscale 7], length 0
>
>
> I don't see anything that looks related in dmesg. The only brcmsmac
> messages I see are:
>
> [ 567.122218] brcmsmac bcma0:0: brcmsmac: brcms_ops_bss_info_changed:
> associated
> [ 567.122226] brcmsmac bcma0:0: brcms_ops_bss_info_changed: arp
> filtering: 1 addresses (implement)
> [ 567.122231] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
> enabled: true (implement)
> [ 567.192283] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
> enabled: true (implement)
>
> I am writing to linux-netdev rather than linux-wireless because
> according to Next/SHA1s the wireless & wireless-next trees were not
> updated between next-20140901 and next-20140902, but the net & net-next
> trees were updated, so maybe the regression came from there. (I haven't
> tested next-20140903 because it won't boot for unrelated reasons.)
>
> Let me know if I should just file this in Bugzilla or what information I
> can provide to help track this down, if it hasn't already been identified.
>
> The here are the good (-) and bad (+) trees from Next/SHA1s at the
> next-* tags mentioned earlier that I built from.
>
> -net 38ab1fa981d543e1b00f4ffbce4ddb480cd2effe
> +net cc25f0cbe4409d6a573b1f3bf7020d5b04076ee9
>
> -net-next dace1b54726bffe1c009f7661e3cee6b762f30c8
> +net-next 364a9e93243d1785f310c0964af0e24bf1adac03
>
Could you post
ethtool -k wlan0
And try
ethtool -K wlan tx off
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Hannes Frederic Sowa @ 2014-09-04 22:45 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eric Dumazet, David Laight, Denis Kirjanov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev, Markos Chandras,
Martin Schwidefsky
In-Reply-To: <CAADnVQL6s_DRE2aMEw=oBdcu6Bn0Q5CfZ_CKJ4xdGdcp5cRV7w@mail.gmail.com>
Hi,
On Thu, Sep 4, 2014, at 22:51, Alexei Starovoitov wrote:
> On Thu, Sep 4, 2014 at 7:41 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > On Do, 2014-09-04 at 07:35 -0700, Eric Dumazet wrote:
> >> On Thu, 2014-09-04 at 16:33 +0200, Hannes Frederic Sowa wrote:
> >>
> >> > Which btw. also uses int, which might change alignment of structures.
> >>
> >> You missed the point .
> >>
> >> kmemcheck wants to make sure the whole word is set, or else you could
> >> get false positives.
> >>
> >> kmemcheck needs are quite different.
> >
> > Now that you said it, I understand. :)
> >
> > You were right with the int vs. u8 thing all along. gcc aligns the
> > datatype on the next struct field and not on the whole field, as I
> > expected. So excuse my error above.
> >
> > I think the latest proposals looks good?
>
> to me: yes
> and I think your latest half-patch with:
> + __u8 __pkt_type_offset[0];
> also looks good.
>
> Are you going to take it over from Denis here?
I don't know. Denis, do you want to incorperate my changes or should I
take over here?
> while at it would you fix sparc jit as well that has comment:
> #if 0
> /* GCC won't let us take the address of
> * a bit field even though we very much
> * know what we are doing here.
> */
> case BPF_ANC | SKF_AD_PKTTYPE:
> __emit_skb_load8(pkt_type, r_A);
> emit_alu_K(SRL, 5);
> break;
> #endif
> should be able to replace 'pkt_type' above with __pkt_type_offset...
I'll have a look then but cannot test sparc easily.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Alexei Starovoitov @ 2014-09-04 21:54 UTC (permalink / raw)
To: vyasevic
Cc: Vlad Yasevich, Jiri Pirko, netdev@vger.kernel.org,
Florian Zumbiehl, Eric Dumazet, Matthew Rosato
In-Reply-To: <5408D318.4020009@redhat.com>
On Thu, Sep 4, 2014 at 2:01 PM, Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 09/04/2014 04:43 PM, Alexei Starovoitov wrote:
>> On Thu, Sep 04, 2014 at 03:29:00PM -0400, Vlad Yasevich wrote:
>>>> nack. This will definitelly break several stacked setups.
>>>
>>> Which ones? The only thing I can see that would behave differently
>>> is something like:
>>>
>>> vlan0 bridge0
>>> | |
>>> +-------- eth0
>>>
>>> In this case, the old code would give an untagged packet to the bridge
>>> and the new code would give a tagged packet.
>>>
>>> This set-up is a bit ambiguous. Remove the vlan, and bridge gets a tagged
>>> traffic even though the vlan has no relationship to the bridge.
>>>
>>> I've tested a couple of different stacked setups and they all seem to work.
>>
>> 2nd nack.
>> It will break user space, including our setup that has:
>> vlanX OVS
>> | |
>> +------ eth0
>>
>> vlan device has IP assigned and all tagged traffic goes through the stack
>> and into control plane process. ovs datapath keeps managing eth0 with
>> all other vlans.
>>
>
> Did you specially configure OVS to pass the traffic up the stack? I see
> OVS will only pass LOOPBACK packets. All others it seems to consume.
>
> Can the same be accomplished with a tagged internal port?
our ovs config is not using internal port. vlan device is used as
control interface and should be independent of ovs datapath.
Theoretically it may be possible to use ovs for both, but very dangerous,
when control and data are going through the same datapath.
Any ovs programming mistake will kill control plane and whole
hypervisor will become inaccessible.
> The reason I am asking, is I am trying to figure out if this is
> a valid config. It seems very hard to get right and seems to work almost
> by accident at times. For example, in the bridge scenario I described.
> vlan and bridge have to share a mac address for that work.
I think it's not valid vs invalid config.
this was the behavior of vlan devices for long time. vlan was parsed
and send to vlan_dev _before_ rx_handler. I suspect there is more
than one user app that is relying on that.
I can change our stuff to do something different, but I think we
should not be breaking vlan behavior for others.
^ permalink raw reply
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: David Miller @ 2014-09-04 21:49 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dborkman-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <CAMEtUuz6zraph_8xXW43bNbh5uPRki0yOufMfV5pKMRLhDrQQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Wed, 3 Sep 2014 22:41:48 -0700
> Do you want me to resubmit just first two?
Yes, you can't submit hodge-podge path series, either it's all to
be applied or it's all RFC material.
^ permalink raw reply
* Re: [PATCH v5 1/4] ethernet: arc: Add support for Rockchip SoC layer device tree bindings
From: Beniamino Galvani @ 2014-09-04 21:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Romain Perier, heiko, linux-rockchip, linux-arm-kernel, netdev,
devicetree
In-Reply-To: <3809311.80Auz8fbHu@wuerfel>
On Thu, Sep 04, 2014 at 11:33:38AM +0200, Arnd Bergmann wrote:
> This is a recurring problem:
>
> driver a depends on x and selects y
> driver b depends on y and selects x
>
> Maybe we should teach Kconfig to not worry about it when x and y are
> both user-selectable as well.
>
> However, a nicer solution would be if we could agree for each symbol
> on who is supposed to 'select' or 'depends on' it. In particular,
> we are inconsistent about CONFIG_REGULATOR:
>
> MDIO_SUN4I probably should not 'select' it but instead 'depends on'
> this symbol if anything. It would be nice if someone could submit
> a patch to that effect. EMAC_ROCKCHIP could also drop the dependency
> on REGULATOR: you can still build the driver without that subsystem
> being enabled, but then all the regulators have to be set up by
> the boot loader.
Thanks, things are clearer now.
If I understand correctly the dependency of MDIO_SUN4I on REGULATOR
can be dropped as well for the same reason; I will send a patch for
that.
Beniamino
>
> There isn't much we can do about the PHYLIB dependency, unless we
> turn it into a silent symbol that gets selected by all phy drivers,
> or we change all network drivers that currently 'select' it to
> 'depends on'. I don't really want to get involved in that discussion ;-)
>
> Arnd
^ permalink raw reply
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Sergei Shtylyov @ 2014-09-04 21:19 UTC (permalink / raw)
To: Vadim Kochan, Vlad Yasevich; +Cc: netdev
In-Reply-To: <CAMw6YJ+G1V4VdcENWs9QhHJgLzYv5j2gb4udJcw5EGPPw89Jhg@mail.gmail.com>
On 09/05/2014 12:01 AM, Vadim Kochan wrote:
> But here http://patchwork.ozlabs.org/project/netdev/list/ I see the v2.
I'm seeing v2 on your posting as well.
> On Thu, Sep 4, 2014 at 10:59 PM, Vadim Kochan <vadim4j@gmail.com> wrote:
>> Hm thats strange I changed subject of the fixed patch to the [PATCH
>> v2] form but it appeared in this thread w/o v2.
The v2 is on its own thread (how it should be).
>> On Thu, Sep 4, 2014 at 10:54 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>> On 09/04/2014 03:35 PM, Vadim Kochan wrote:
>>>> Thanks!
>>>> Should I re-send the fixed patch with some special subject after
>>>> comments/review ?
>>> Typically a [PATCH v2] and so on if there are comments on v2.
>>> -vlad
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Vlad Yasevich @ 2014-09-04 21:01 UTC (permalink / raw)
To: Alexei Starovoitov, Vlad Yasevich
Cc: Jiri Pirko, netdev, Florian Zumbiehl, Eric Dumazet,
Matthew Rosato
In-Reply-To: <20140904204314.GA16612@its-macbook-pro.plumgrid.com>
On 09/04/2014 04:43 PM, Alexei Starovoitov wrote:
> On Thu, Sep 04, 2014 at 03:29:00PM -0400, Vlad Yasevich wrote:
>>> nack. This will definitelly break several stacked setups.
>>
>> Which ones? The only thing I can see that would behave differently
>> is something like:
>>
>> vlan0 bridge0
>> | |
>> +-------- eth0
>>
>> In this case, the old code would give an untagged packet to the bridge
>> and the new code would give a tagged packet.
>>
>> This set-up is a bit ambiguous. Remove the vlan, and bridge gets a tagged
>> traffic even though the vlan has no relationship to the bridge.
>>
>> I've tested a couple of different stacked setups and they all seem to work.
>
> 2nd nack.
> It will break user space, including our setup that has:
> vlanX OVS
> | |
> +------ eth0
>
> vlan device has IP assigned and all tagged traffic goes through the stack
> and into control plane process. ovs datapath keeps managing eth0 with
> all other vlans.
>
Did you specially configure OVS to pass the traffic up the stack? I see
OVS will only pass LOOPBACK packets. All others it seems to consume.
Can the same be accomplished with a tagged internal port?
The reason I am asking, is I am trying to figure out if this is
a valid config. It seems very hard to get right and seems to work almost
by accident at times. For example, in the bridge scenario I described.
vlan and bridge have to share a mac address for that work.
-vlad
^ permalink raw reply
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Alexei Starovoitov @ 2014-09-04 20:51 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, David Laight, Denis Kirjanov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev@vger.kernel.org,
Markos Chandras, Martin Schwidefsky
In-Reply-To: <1409841709.23465.25.camel@localhost>
On Thu, Sep 4, 2014 at 7:41 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Do, 2014-09-04 at 07:35 -0700, Eric Dumazet wrote:
>> On Thu, 2014-09-04 at 16:33 +0200, Hannes Frederic Sowa wrote:
>>
>> > Which btw. also uses int, which might change alignment of structures.
>>
>> You missed the point .
>>
>> kmemcheck wants to make sure the whole word is set, or else you could
>> get false positives.
>>
>> kmemcheck needs are quite different.
>
> Now that you said it, I understand. :)
>
> You were right with the int vs. u8 thing all along. gcc aligns the
> datatype on the next struct field and not on the whole field, as I
> expected. So excuse my error above.
>
> I think the latest proposals looks good?
to me: yes
and I think your latest half-patch with:
+ __u8 __pkt_type_offset[0];
also looks good.
Are you going to take it over from Denis here?
while at it would you fix sparc jit as well that has comment:
#if 0
/* GCC won't let us take the address of
* a bit field even though we very much
* know what we are doing here.
*/
case BPF_ANC | SKF_AD_PKTTYPE:
__emit_skb_load8(pkt_type, r_A);
emit_alu_K(SRL, 5);
break;
#endif
should be able to replace 'pkt_type' above with __pkt_type_offset...
^ permalink raw reply
* Re: [patch net-next 01/13] openvswitch: split flow structures into ovs specific and generic ones
From: Pravin Shelar @ 2014-09-04 20:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: ryazanov.s.a, jasowang, john.r.fastabend, Neil Jerram,
Eric Dumazet, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, nbd,
Florian Fainelli, Rony Efraim, jeffrey.t.kirsher, Or Gerlitz,
Ben Hutchings, buytenh, roopa, Jamal Hadi Salim, aviadr,
Nicolas Dichtel, vyasevic, nhorman, netdev, Stephen Hemminger,
Daniel Borkmann, ebiederm <eb
In-Reply-To: <20140904123323.GF1867-6KJVSR23iU5sFDB2n11ItA@public.gmane.org>
On Thu, Sep 4, 2014 at 5:33 AM, Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org> wrote:
> Wed, Sep 03, 2014 at 08:41:39PM CEST, pshelar-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org wrote:
>>On Wed, Sep 3, 2014 at 2:24 AM, Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org> wrote:
>>> After this, flow related structures can be used in other code.
>>>
>>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>> ---
>>> include/net/sw_flow.h | 99 ++++++++++++++++++++++++++++++++++
>>> net/openvswitch/actions.c | 3 +-
>>> net/openvswitch/datapath.c | 74 +++++++++++++-------------
>>> net/openvswitch/datapath.h | 4 +-
>>> net/openvswitch/flow.c | 6 +--
>>> net/openvswitch/flow.h | 102 +++++++----------------------------
>>> net/openvswitch/flow_netlink.c | 53 +++++++++---------
>>> net/openvswitch/flow_netlink.h | 10 ++--
>>> net/openvswitch/flow_table.c | 118 ++++++++++++++++++++++-------------------
>>> net/openvswitch/flow_table.h | 30 +++++------
>>> net/openvswitch/vport-gre.c | 4 +-
>>> net/openvswitch/vport-vxlan.c | 2 +-
>>> net/openvswitch/vport.c | 2 +-
>>> net/openvswitch/vport.h | 2 +-
>>> 14 files changed, 276 insertions(+), 233 deletions(-)
>>> create mode 100644 include/net/sw_flow.h
>>>
>>> diff --git a/include/net/sw_flow.h b/include/net/sw_flow.h
>>> new file mode 100644
>>> index 0000000..21724f1
>>> --- /dev/null
>>> +++ b/include/net/sw_flow.h
>>> @@ -0,0 +1,99 @@
>>> +/*
>>> + * include/net/sw_flow.h - Generic switch flow structures
>>> + * Copyright (c) 2007-2012 Nicira, Inc.
>>> + * Copyright (c) 2014 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + */
>>> +
>>> +#ifndef _NET_SW_FLOW_H_
>>> +#define _NET_SW_FLOW_H_
>>> +
>>> +struct sw_flow_key_ipv4_tunnel {
>>> + __be64 tun_id;
>>> + __be32 ipv4_src;
>>> + __be32 ipv4_dst;
>>> + __be16 tun_flags;
>>> + u8 ipv4_tos;
>>> + u8 ipv4_ttl;
>>> +};
>>> +
>>> +struct sw_flow_key {
>>> + struct sw_flow_key_ipv4_tunnel tun_key; /* Encapsulating tunnel key. */
>>> + struct {
>>> + u32 priority; /* Packet QoS priority. */
>>> + u32 skb_mark; /* SKB mark. */
>>> + u16 in_port; /* Input switch port (or DP_MAX_PORTS). */
>>> + } __packed phy; /* Safe when right after 'tun_key'. */
>>> + struct {
>>> + u8 src[ETH_ALEN]; /* Ethernet source address. */
>>> + u8 dst[ETH_ALEN]; /* Ethernet destination address. */
>>> + __be16 tci; /* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
>>> + __be16 type; /* Ethernet frame type. */
>>> + } eth;
>>> + struct {
>>> + u8 proto; /* IP protocol or lower 8 bits of ARP opcode. */
>>> + u8 tos; /* IP ToS. */
>>> + u8 ttl; /* IP TTL/hop limit. */
>>> + u8 frag; /* One of OVS_FRAG_TYPE_*. */
>>> + } ip;
>>> + struct {
>>> + __be16 src; /* TCP/UDP/SCTP source port. */
>>> + __be16 dst; /* TCP/UDP/SCTP destination port. */
>>> + __be16 flags; /* TCP flags. */
>>> + } tp;
>>> + union {
>>> + struct {
>>> + struct {
>>> + __be32 src; /* IP source address. */
>>> + __be32 dst; /* IP destination address. */
>>> + } addr;
>>> + struct {
>>> + u8 sha[ETH_ALEN]; /* ARP source hardware address. */
>>> + u8 tha[ETH_ALEN]; /* ARP target hardware address. */
>>> + } arp;
>>> + } ipv4;
>>> + struct {
>>> + struct {
>>> + struct in6_addr src; /* IPv6 source address. */
>>> + struct in6_addr dst; /* IPv6 destination address. */
>>> + } addr;
>>> + __be32 label; /* IPv6 flow label. */
>>> + struct {
>>> + struct in6_addr target; /* ND target address. */
>>> + u8 sll[ETH_ALEN]; /* ND source link layer address. */
>>> + u8 tll[ETH_ALEN]; /* ND target link layer address. */
>>> + } nd;
>>> + } ipv6;
>>> + };
>>> +} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
>>> +
>>
>>HW offload API should be separate from OVS module. This has following
>>advantages.
>>1. It can be managed by OVS userspace vswitchd process which has much
>>better context to setup hardware flow table. Once we add capabilities
>>for swdev, it is much more easier for vswitchd process to choose
>>correct (hw or sw) flow table for given flow.
>
> The idea is to add a nl attr in ovs genl iface so the vswitchd can
> speficify the flow the to be in sw only, in hw only, in both.
> I believe that is is more convenient to let switchd to communicate flows
> via single iface.
>
How is it convenient? this patch complicates OVS kernel module. It add
OVS interfaces for HW offload. And you need similar interfaces for
switchdev device. So it duplicate code.
On the other hand if vswitchd uses common interface (switchdev) there
is no need to extend ovs kernel interface. For example specifying
extra metadata, like (sw only, hw olny, both).
>>2. Other application that wants to use HW offload does not have
>>dependency on OVS kernel module.
>
> That is not the case for this patchset. Userspace can insert/remove
> flows using the switchdev generic netlink api - see:
> [patch net-next 13/13] switchdev: introduce Netlink API
>
>>3. Hardware and software datapath remains separate, these two
>>components has no dependency on each other, both can be developed
>>independent of each other.
>
>
> The general idea is to have the offloads handled in-kernel. Therefore I
> hooked on to ovs kernel dp code.
>
>
>
^ permalink raw reply
* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Alexei Starovoitov @ 2014-09-04 20:43 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Jiri Pirko, netdev, Vladislav Yasevich, Florian Zumbiehl,
Eric Dumazet, Matthew Rosato
In-Reply-To: <5408BD7C.7030805@gmail.com>
On Thu, Sep 04, 2014 at 03:29:00PM -0400, Vlad Yasevich wrote:
> > nack. This will definitelly break several stacked setups.
>
> Which ones? The only thing I can see that would behave differently
> is something like:
>
> vlan0 bridge0
> | |
> +-------- eth0
>
> In this case, the old code would give an untagged packet to the bridge
> and the new code would give a tagged packet.
>
> This set-up is a bit ambiguous. Remove the vlan, and bridge gets a tagged
> traffic even though the vlan has no relationship to the bridge.
>
> I've tested a couple of different stacked setups and they all seem to work.
2nd nack.
It will break user space, including our setup that has:
vlanX OVS
| |
+------ eth0
vlan device has IP assigned and all tagged traffic goes through the stack
and into control plane process. ovs datapath keeps managing eth0 with
all other vlans.
^ permalink raw reply
* [PATCH] ip monitor: Skip IPv6 ND user option messages
From: Vadim Kochan @ 2014-09-04 20:24 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
IPv6 router sends ND messages with RDNSS option
which causes the printing of unknown message by 'ip monitor':
Unknown message: 0000004c 00000044 00000000
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
ip/ipmonitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 70f2a7a..0029b52 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -131,7 +131,8 @@ static int accept_msg(const struct sockaddr_nl *who,
n->nlmsg_type == RTM_NEWTCLASS ||
n->nlmsg_type == RTM_DELTCLASS ||
n->nlmsg_type == RTM_NEWTFILTER ||
- n->nlmsg_type == RTM_DELTFILTER)
+ n->nlmsg_type == RTM_DELTFILTER ||
+ n->nlmsg_type == RTM_NEWNDUSEROPT)
return 0;
if (n->nlmsg_type != NLMSG_ERROR && n->nlmsg_type != NLMSG_NOOP &&
n->nlmsg_type != NLMSG_DONE) {
--
2.1.0
^ permalink raw reply related
* Regression: TCP connections fail over wireless: bad cksum?
From: Ted Percival @ 2014-09-04 20:11 UTC (permalink / raw)
To: netdev
Yesterday's linux-next build introduced a problem with wireless
networking on my machine. ie. next-20140901 worked fine but
next-20140902 does not seem able to sustain a TCP connection over
wireless. Wired networking works fine. I am using the brcmsmac driver
and the hardware is "Broadcom Corporation BCM4313 802.11b/g/n Wireless
LAN Controller (rev 01)".
Pings, even large pings (ping -s 16000) work fine but TCP connections hang.
I looked through the changes between the bad & good commits from the net
& net-next trees and I wonder if some of the changes to checksumming
have surfaced a problem with this driver. When I look at a tcpdump, it
indicates that all the checksums are wrong (although I don't know if
that is just due to hardware offload).
Here is a short trace of the hung connection attempt of
curl http://lwn.net/
$ sudo tcpdump -vvn -i wlan0 port 80
tcpdump: listening on wlan0, link-type EN10MB (Ethernet), capture size
65535 bytes
13:20:15.120770 IP (tos 0x0, ttl 64, id 22734, offset 0, flags [DF],
proto TCP (6), length 60)
10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
(incorrect -> 0x4fac), seq 2861986597, win 29200, options [mss
1460,sackOK,TS val 142315 ecr 0,nop,wscale 7], length 0
13:20:16.121755 IP (tos 0x0, ttl 64, id 22735, offset 0, flags [DF],
proto TCP (6), length 60)
10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
(incorrect -> 0x4bc3), seq 2861986597, win 29200, options [mss
1460,sackOK,TS val 143316 ecr 0,nop,wscale 7], length 0
13:20:18.125748 IP (tos 0x0, ttl 64, id 22736, offset 0, flags [DF],
proto TCP (6), length 60)
10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
(incorrect -> 0x43ef), seq 2861986597, win 29200, options [mss
1460,sackOK,TS val 145320 ecr 0,nop,wscale 7], length 0
13:20:22.133743 IP (tos 0x0, ttl 64, id 22737, offset 0, flags [DF],
proto TCP (6), length 60)
10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
(incorrect -> 0x3447), seq 2861986597, win 29200, options [mss
1460,sackOK,TS val 149328 ecr 0,nop,wscale 7], length 0
13:20:30.149754 IP (tos 0x0, ttl 64, id 22738, offset 0, flags [DF],
proto TCP (6), length 60)
10.5.51.93.38035 > 72.51.34.34.80: Flags [S], cksum 0xa7e5
(incorrect -> 0x14f7), seq 2861986597, win 29200, options [mss
1460,sackOK,TS val 157344 ecr 0,nop,wscale 7], length 0
I don't see anything that looks related in dmesg. The only brcmsmac
messages I see are:
[ 567.122218] brcmsmac bcma0:0: brcmsmac: brcms_ops_bss_info_changed:
associated
[ 567.122226] brcmsmac bcma0:0: brcms_ops_bss_info_changed: arp
filtering: 1 addresses (implement)
[ 567.122231] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
enabled: true (implement)
[ 567.192283] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos
enabled: true (implement)
I am writing to linux-netdev rather than linux-wireless because
according to Next/SHA1s the wireless & wireless-next trees were not
updated between next-20140901 and next-20140902, but the net & net-next
trees were updated, so maybe the regression came from there. (I haven't
tested next-20140903 because it won't boot for unrelated reasons.)
Let me know if I should just file this in Bugzilla or what information I
can provide to help track this down, if it hasn't already been identified.
The here are the good (-) and bad (+) trees from Next/SHA1s at the
next-* tags mentioned earlier that I built from.
-net 38ab1fa981d543e1b00f4ffbce4ddb480cd2effe
+net cc25f0cbe4409d6a573b1f3bf7020d5b04076ee9
-net-next dace1b54726bffe1c009f7661e3cee6b762f30c8
+net-next 364a9e93243d1785f310c0964af0e24bf1adac03
--
Cheers,
-Ted
^ permalink raw reply
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vadim Kochan @ 2014-09-04 20:01 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Sergei Shtylyov, netdev
In-Reply-To: <CAMw6YJJRfBLh0Jft=zCN8tj8wm6qhjoB=NibJwtcn8AqajXmXw@mail.gmail.com>
But here http://patchwork.ozlabs.org/project/netdev/list/ I see the v2.
On Thu, Sep 4, 2014 at 10:59 PM, Vadim Kochan <vadim4j@gmail.com> wrote:
> Hm thats strange I changed subject of the fixed patch to the [PATCH
> v2] form but it appeared in this thread w/o v2.
>
> On Thu, Sep 4, 2014 at 10:54 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
>> On 09/04/2014 03:35 PM, Vadim Kochan wrote:
>>> Thanks!
>>>
>>> Should I re-send the fixed patch with some special subject after
>>> comments/review ?
>>
>> Typically a [PATCH v2] and so on if there are comments on v2.
>>
>> -vlad
>>
>>>
>>> On Thu, Sep 4, 2014 at 10:29 PM, Sergei Shtylyov
>>> <sergei.shtylyov@cogentembedded.com> wrote:
>>>> Hello.
>>>>
>>>>
>>>> On 09/04/2014 10:52 PM, Vadim Kochan wrote:
>>>>
>>>>> This is ugly fix but solves the case when timestamp
>>>>> or banner-label is printed before the cloned route will be skipped
>>>>> by iproute filter which filters out all cached routes by default.
>>>>> In such case timestamp will be printed twice:
>>>>
>>>>
>>>>> Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
>>>>> Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
>>>>> 10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
>>>>
>>>>
>>>>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
>>>>> ---
>>>>> ip/ipmonitor.c | 13 ++++++++++---
>>>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>>
>>>>
>>>>> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
>>>>> index 70f2a7a..2bd3fed 100644
>>>>> --- a/ip/ipmonitor.c
>>>>> +++ b/ip/ipmonitor.c
>>>>
>>>> [...]
>>>>
>>>>> @@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
>>>>> return -1;
>>>>> }
>>>>>
>>>>> + if (r->rtm_flags&RTM_F_CLONED)
>>>>
>>>>
>>>> Judging on the patched code, spaces are needed around &.
>>>>
>>>>
>>>>> + return 0;
>>>>> +
>>>>> + if (timestamp)
>>>>> + print_timestamp(fp);
>>>>> +
>>>>> if (r->rtm_family == RTNL_FAMILY_IPMR ||
>>>>> r->rtm_family == RTNL_FAMILY_IP6MR) {
>>>>> if (prefix_banner)
>>>>
>>>> [...]
>>>>
>>>> WBR, Sergei
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
^ permalink raw reply
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vadim Kochan @ 2014-09-04 19:59 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Sergei Shtylyov, netdev
In-Reply-To: <5408C37C.6050104@gmail.com>
Hm thats strange I changed subject of the fixed patch to the [PATCH
v2] form but it appeared in this thread w/o v2.
On Thu, Sep 4, 2014 at 10:54 PM, Vlad Yasevich <vyasevich@gmail.com> wrote:
> On 09/04/2014 03:35 PM, Vadim Kochan wrote:
>> Thanks!
>>
>> Should I re-send the fixed patch with some special subject after
>> comments/review ?
>
> Typically a [PATCH v2] and so on if there are comments on v2.
>
> -vlad
>
>>
>> On Thu, Sep 4, 2014 at 10:29 PM, Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com> wrote:
>>> Hello.
>>>
>>>
>>> On 09/04/2014 10:52 PM, Vadim Kochan wrote:
>>>
>>>> This is ugly fix but solves the case when timestamp
>>>> or banner-label is printed before the cloned route will be skipped
>>>> by iproute filter which filters out all cached routes by default.
>>>> In such case timestamp will be printed twice:
>>>
>>>
>>>> Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
>>>> Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
>>>> 10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
>>>
>>>
>>>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
>>>> ---
>>>> ip/ipmonitor.c | 13 ++++++++++---
>>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>>>
>>>> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
>>>> index 70f2a7a..2bd3fed 100644
>>>> --- a/ip/ipmonitor.c
>>>> +++ b/ip/ipmonitor.c
>>>
>>> [...]
>>>
>>>> @@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
>>>> return -1;
>>>> }
>>>>
>>>> + if (r->rtm_flags&RTM_F_CLONED)
>>>
>>>
>>> Judging on the patched code, spaces are needed around &.
>>>
>>>
>>>> + return 0;
>>>> +
>>>> + if (timestamp)
>>>> + print_timestamp(fp);
>>>> +
>>>> if (r->rtm_family == RTNL_FAMILY_IPMR ||
>>>> r->rtm_family == RTNL_FAMILY_IP6MR) {
>>>> if (prefix_banner)
>>>
>>> [...]
>>>
>>> WBR, Sergei
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply
* [PATCH v2] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vadim Kochan @ 2014-09-04 19:48 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
This is ugly fix but solves the case when timestamp
or banner-label is printed before the cloned route will be skipped
by iproute filter which filters out all cached routes by default.
In such case timestamp will be printed twice:
Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
ip/ipmonitor.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
index 70f2a7a..3cdbe1b 100644
--- a/ip/ipmonitor.c
+++ b/ip/ipmonitor.c
@@ -41,9 +41,6 @@ static int accept_msg(const struct sockaddr_nl *who,
{
FILE *fp = (FILE*)arg;
- if (timestamp)
- print_timestamp(fp);
-
if (n->nlmsg_type == RTM_NEWROUTE || n->nlmsg_type == RTM_DELROUTE) {
struct rtmsg *r = NLMSG_DATA(n);
int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
@@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
return -1;
}
+ if (r->rtm_flags & RTM_F_CLONED)
+ return 0;
+
+ if (timestamp)
+ print_timestamp(fp);
+
if (r->rtm_family == RTNL_FAMILY_IPMR ||
r->rtm_family == RTNL_FAMILY_IP6MR) {
if (prefix_banner)
@@ -66,6 +69,10 @@ static int accept_msg(const struct sockaddr_nl *who,
return 0;
}
}
+
+ if (timestamp)
+ print_timestamp(fp);
+
if (n->nlmsg_type == RTM_NEWLINK || n->nlmsg_type == RTM_DELLINK) {
ll_remember_index(who, n, NULL);
if (prefix_banner)
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vlad Yasevich @ 2014-09-04 19:54 UTC (permalink / raw)
To: Vadim Kochan, Sergei Shtylyov; +Cc: netdev
In-Reply-To: <CAMw6YJL_-6_zXo9tmEz3M8V-CxeoaAN_y0wscDt+JsM8xKkqAw@mail.gmail.com>
On 09/04/2014 03:35 PM, Vadim Kochan wrote:
> Thanks!
>
> Should I re-send the fixed patch with some special subject after
> comments/review ?
Typically a [PATCH v2] and so on if there are comments on v2.
-vlad
>
> On Thu, Sep 4, 2014 at 10:29 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>> Hello.
>>
>>
>> On 09/04/2014 10:52 PM, Vadim Kochan wrote:
>>
>>> This is ugly fix but solves the case when timestamp
>>> or banner-label is printed before the cloned route will be skipped
>>> by iproute filter which filters out all cached routes by default.
>>> In such case timestamp will be printed twice:
>>
>>
>>> Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
>>> Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
>>> 10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
>>
>>
>>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
>>> ---
>>> ip/ipmonitor.c | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>>
>>> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
>>> index 70f2a7a..2bd3fed 100644
>>> --- a/ip/ipmonitor.c
>>> +++ b/ip/ipmonitor.c
>>
>> [...]
>>
>>> @@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
>>> return -1;
>>> }
>>>
>>> + if (r->rtm_flags&RTM_F_CLONED)
>>
>>
>> Judging on the patched code, spaces are needed around &.
>>
>>
>>> + return 0;
>>> +
>>> + if (timestamp)
>>> + print_timestamp(fp);
>>> +
>>> if (r->rtm_family == RTNL_FAMILY_IPMR ||
>>> r->rtm_family == RTNL_FAMILY_IP6MR) {
>>> if (prefix_banner)
>>
>> [...]
>>
>> WBR, Sergei
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Vadim Kochan @ 2014-09-04 19:35 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev
In-Reply-To: <5408BD9F.9060107@cogentembedded.com>
Thanks!
Should I re-send the fixed patch with some special subject after
comments/review ?
On Thu, Sep 4, 2014 at 10:29 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 09/04/2014 10:52 PM, Vadim Kochan wrote:
>
>> This is ugly fix but solves the case when timestamp
>> or banner-label is printed before the cloned route will be skipped
>> by iproute filter which filters out all cached routes by default.
>> In such case timestamp will be printed twice:
>
>
>> Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
>> Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
>> 10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
>
>
>> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
>> ---
>> ip/ipmonitor.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>
>
>> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
>> index 70f2a7a..2bd3fed 100644
>> --- a/ip/ipmonitor.c
>> +++ b/ip/ipmonitor.c
>
> [...]
>
>> @@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
>> return -1;
>> }
>>
>> + if (r->rtm_flags&RTM_F_CLONED)
>
>
> Judging on the patched code, spaces are needed around &.
>
>
>> + return 0;
>> +
>> + if (timestamp)
>> + print_timestamp(fp);
>> +
>> if (r->rtm_family == RTNL_FAMILY_IPMR ||
>> r->rtm_family == RTNL_FAMILY_IP6MR) {
>> if (prefix_banner)
>
> [...]
>
> WBR, Sergei
>
^ permalink raw reply
* Re: [PATCH v5 1/4] ethernet: arc: Add support for Rockchip SoC layer device tree bindings
From: Beniamino Galvani @ 2014-09-04 19:30 UTC (permalink / raw)
To: PERIER Romain
Cc: Heiko Stübner, linux-rockchip, linux-arm-kernel, netdev,
devicetree, Arnd Bergmann
In-Reply-To: <CABgxDoLWqqwGUUt9hWuJdHT_VF0c6-7xeAwgfeH7TwJmu1BN9w@mail.gmail.com>
On Thu, Sep 04, 2014 at 10:45:35AM +0200, PERIER Romain wrote:
> Hi Beniamino,
>
> I will investigate. Could you please :
>
> - Load the module
> - Show me the output of lsmod
> - Use your ethernet a bit
> - Try to unload the module
> - If your board did not freeze or if your kernel did not panic , show
> me your dmesg (if it is impossible show me the dmesg before unloading
> the module)
>
> Perhaps, we could continue this discussion in private (by email or
> IRC). What do you think ?
>
> I will investigate this evening and tomorrow. Thanks for your feedbacks .
Hi Romain,
the freeze happens in clk_disable_unprepare(), which disables the
macref clock parents up to the dpll, needed by the DDR.
As suggested by Heiko on IRC, the patch "clk: rockchip: protect
critical clocks from getting disabled" [1], modified to include ddrphy
in the list of critical clocks, solves the problem.
During the module unload, if you move the call to arc_emac_remove()
before clk_disable_unprepare() you can avoid a delay of 4-5 seconds
caused by the timeout of the function phy_disconnect(). In any case:
Tested-by: Beniamino Galvani <b.galvani@gmail.com>
[1] https://git.linaro.org/people/mike.turquette/linux.git/commit/fe94f974e9c8b820640a5873d81589ab67380516
>
>
> 2014-09-03 23:12 GMT+02:00 Beniamino Galvani <b.galvani@gmail.com>:
> > On Wed, Sep 03, 2014 at 04:52:42PM +0000, 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.
> >
> > Hi Romain,
> >
> > on a Radxa Rock when I try to remove the emac_rockchip module the
> > board locks up when calling clk_disable_unprepare(priv->refclk). The
> > tree is a net-next + your series, I don't know if I need some other
> > patches.
> >
> > There is also the following build warning due to the emac dependency
> > on REGULATOR which in principle seems correct, but looking at other
> > drivers I wonder why they use the regulator APIs but don't have the
> > same dependency.
> >
> > drivers/regulator/Kconfig:1:error: recursive dependency detected!
> > drivers/regulator/Kconfig:1: symbol REGULATOR is selected by MDIO_SUN4I
> > drivers/net/phy/Kconfig:159: symbol MDIO_SUN4I depends on PHYLIB
> > drivers/net/phy/Kconfig:5: symbol PHYLIB is selected by ARC_EMAC_CORE
> > drivers/net/ethernet/arc/Kconfig:20: symbol ARC_EMAC_CORE is selected by EMAC_ROCKCHIP
> > drivers/net/ethernet/arc/Kconfig:35: symbol EMAC_ROCKCHIP depends on REGULATOR
> >
> > Regards,
> > Beniamino
^ permalink raw reply
* Re: [PATCH] ip monitor: Dont print timestamp or banner-label for cloned routes
From: Sergei Shtylyov @ 2014-09-04 19:29 UTC (permalink / raw)
To: Vadim Kochan, netdev
In-Reply-To: <1409856738-1938-1-git-send-email-vadim4j@gmail.com>
Hello.
On 09/04/2014 10:52 PM, Vadim Kochan wrote:
> This is ugly fix but solves the case when timestamp
> or banner-label is printed before the cloned route will be skipped
> by iproute filter which filters out all cached routes by default.
> In such case timestamp will be printed twice:
> Timestamp: Thu Sep 4 19:46:59 2014 457933 usec
> Timestamp: Thu Sep 4 19:47:07 2014 977970 usec
> 10.3.5.1 dev wlp3s0 lladdr XX:XX:XX:XX:XX:XX STALE
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
> ip/ipmonitor.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
> index 70f2a7a..2bd3fed 100644
> --- a/ip/ipmonitor.c
> +++ b/ip/ipmonitor.c
[...]
> @@ -53,6 +50,12 @@ static int accept_msg(const struct sockaddr_nl *who,
> return -1;
> }
>
> + if (r->rtm_flags&RTM_F_CLONED)
Judging on the patched code, spaces are needed around &.
> + return 0;
> +
> + if (timestamp)
> + print_timestamp(fp);
> +
> if (r->rtm_family == RTNL_FAMILY_IPMR ||
> r->rtm_family == RTNL_FAMILY_IP6MR) {
> if (prefix_banner)
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Vlad Yasevich @ 2014-09-04 19:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, Vladislav Yasevich, Florian Zumbiehl, Eric Dumazet,
Matthew Rosato
In-Reply-To: <20140904190524.GA1938@nanopsycho.lan>
On 09/04/2014 03:05 PM, Jiri Pirko wrote:
> Thu, Sep 04, 2014 at 08:40:43PM CEST, vyasevich@gmail.com wrote:
>> Currently, we attempt to remove the vlan informaion from the packet
>> before passing it to the rx_handler. In most situations this works
>> just fine since the rx_handlers are usually installed for the
>> lower device and thus vlan device isn't found. However, macvtap
>> device is a bit different as it installs an rx_handler on top
>> of a macvlan device. As a result, if someone was define a vlan
>> device on top of a macvap (for the purposes of enabling a VM
>> to use vlans with macvtap), then the current code will result
>> in passing an untagged packet to the macvtap rx_handler and the
>> VM will not receive tagged traffic.
>
> skb->vlan_tci is set. macvlan should work with that to pass the frame
> correctly. This should be handled in macvtap code.
OK. Consider a configuration.
vlan10
vlan10 |
| VM1:eth0
v |
macvtap0 <---+
|
V
eth0
On the host, vlan10 can't really send and receive traffic. It's only
there to add a vlan filter to eth0 so that packets marked with vlan10
can be received.
When traffic is processed by __netif_receive_skb_core(), skb->dev is eth0 and we
have the tci, so that when vlan_do_receive() is called, we can't find the vlan
device and call the rx_handler macvlan_handle_frame().
That handler calls netif_rx() with skb->dev set to macvtap0.
This time through the receive path, vlan_tci is still set and we
do find the vlan device which is on top of macvtap0, so we set the tci to 0
and then pass it to the rx_handler macvtap_handle_frame().
As a result, we pass an untagged frame to the VM.
>
> btw can you give me an example of setup where rx_handler is set for
> macvlan device? I wonder what is could be.
>
>>
>> This patch moves the untaggin code to after the rx_handler for
>> the current device has been called. This still works for the
>> existing rx_handlers (like bonding/teaming/bridging/etc) and
>> also makes vlans on top of macvtap work as before.
>>
>> Fixes: 6acf54f1cf (macvtap: Add support of packet capture on macvtap device)
>> Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>> ---
>> net/core/dev.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index ab9a165..54691d1 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3642,17 +3642,6 @@ ncls:
>> if (pfmemalloc && !skb_pfmemalloc_protocol(skb))
>> goto drop;
>>
>> - if (vlan_tx_tag_present(skb)) {
>> - if (pt_prev) {
>> - ret = deliver_skb(skb, pt_prev, orig_dev);
>> - pt_prev = NULL;
>> - }
>> - if (vlan_do_receive(&skb))
>> - goto another_round;
>> - else if (unlikely(!skb))
>> - goto unlock;
>> - }
>> -
>> rx_handler = rcu_dereference(skb->dev->rx_handler);
>> if (rx_handler) {
>> if (pt_prev) {
>> @@ -3674,6 +3663,17 @@ ncls:
>> }
>> }
>>
>> + if (vlan_tx_tag_present(skb)) {
>> + if (pt_prev) {
>> + ret = deliver_skb(skb, pt_prev, orig_dev);
>> + pt_prev = NULL;
>> + }
>> + if (vlan_do_receive(&skb))
>> + goto another_round;
>> + else if (unlikely(!skb))
>> + goto unlock;
>> + }
>> +
>
> nack. This will definitelly break several stacked setups.
Which ones? The only thing I can see that would behave differently
is something like:
vlan0 bridge0
| |
+-------- eth0
In this case, the old code would give an untagged packet to the bridge
and the new code would give a tagged packet.
This set-up is a bit ambiguous. Remove the vlan, and bridge gets a tagged
traffic even though the vlan has no relationship to the bridge.
I've tested a couple of different stacked setups and they all seem to work.
Thanks
-vlad
>
>
>> if (unlikely(vlan_tx_tag_present(skb))) {
>> if (vlan_tx_tag_get_id(skb))
>> skb->pkt_type = PACKET_OTHERHOST;
>> --
>> 1.9.3
>>
^ permalink raw reply
* Re: [PATCH net-next 12/12] be2net: query max_tx_qs for BE3 super-nic profile from FW
From: Jiri Pirko @ 2014-09-04 19:28 UTC (permalink / raw)
To: Sathya Perla; +Cc: Sergei Shtylyov, netdev@vger.kernel.org
In-Reply-To: <CF9D1877D81D214CB0CA0669EFAE020C56D414CF@CMEXMB1.ad.emulex.com>
Tue, Sep 02, 2014 at 06:08:18AM CEST, Sathya.Perla@Emulex.Com wrote:
>> -----Original Message-----
>> From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
>>
>> Hello.
>>
>> > In the BE3 super-nic profile, the max_tx_qs value can vary for each
>> function.
>> > So the driver needs to query this value from FW instead of using the
>> > pre-defined constant BE3_MAX_TX_QS.
>>
>> > Signed-off-by: Suresh Reddy <Suresh.Reddy@emulex.com>
>> > Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
>> > ---
>> > drivers/net/ethernet/emulex/benet/be_main.c | 13 +++++++++++--
>> > 1 files changed, 11 insertions(+), 2 deletions(-)
>>
>> > diff --git a/drivers/net/ethernet/emulex/benet/be_main.c
>> b/drivers/net/ethernet/emulex/benet/be_main.c
>> > index f3235d1..a30156b 100644
>> > --- a/drivers/net/ethernet/emulex/benet/be_main.c
>> > +++ b/drivers/net/ethernet/emulex/benet/be_main.c
>> > @@ -3324,10 +3324,19 @@ static void BEx_get_resources(struct
>> be_adapter *adapter,
>> > */
>> > if (BE2_chip(adapter) || use_sriov || (adapter->port_num > 1) ||
>> > !be_physfn(adapter) || (be_is_mc(adapter) &&
>> > - !(adapter->function_caps & BE_FUNCTION_CAPS_RSS)))
>> > + !(adapter->function_caps & BE_FUNCTION_CAPS_RSS))) {
>> > res->max_tx_qs = 1;
>> > - else
>> > + } else if (adapter->function_caps &
>> BE_FUNCTION_CAPS_SUPER_NIC) {
>> > + struct be_resources super_nic_res = {0};
>>
>> Empty line is needed after declaration.
>Ok, will fix it. Thks.
scripts/checkpatch.pl is your friend :)
>
>>
>> > + /* On a SuperNIC profile, the driver needs to use the
>> > + * GET_PROFILE_CONFIG cmd to query the per-function TXQ
>> limits
>> > + */
>> > + be_cmd_get_profile_config(adapter, &super_nic_res, 0);
>> > + /* Some old versions of BE3 FW don't report max_tx_qs
>> value */
>> > + res->max_tx_qs = super_nic_res.max_tx_qs ? :
>> BE3_MAX_TX_QS;
>> > + } else {
>> > res->max_tx_qs = BE3_MAX_TX_QS;
>> > + }
>>
>> WBR, Sergei
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] core: Untag packets after rx_handler has run.
From: Jiri Pirko @ 2014-09-04 19:05 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: netdev, Vladislav Yasevich, Florian Zumbiehl, Eric Dumazet,
Matthew Rosato
In-Reply-To: <1409856043-21840-1-git-send-email-vyasevic@redhat.com>
Thu, Sep 04, 2014 at 08:40:43PM CEST, vyasevich@gmail.com wrote:
>Currently, we attempt to remove the vlan informaion from the packet
>before passing it to the rx_handler. In most situations this works
>just fine since the rx_handlers are usually installed for the
>lower device and thus vlan device isn't found. However, macvtap
>device is a bit different as it installs an rx_handler on top
>of a macvlan device. As a result, if someone was define a vlan
>device on top of a macvap (for the purposes of enabling a VM
>to use vlans with macvtap), then the current code will result
>in passing an untagged packet to the macvtap rx_handler and the
>VM will not receive tagged traffic.
skb->vlan_tci is set. macvlan should work with that to pass the frame
correctly. This should be handled in macvtap code.
btw can you give me an example of setup where rx_handler is set for
macvlan device? I wonder what is could be.
>
>This patch moves the untaggin code to after the rx_handler for
>the current device has been called. This still works for the
>existing rx_handlers (like bonding/teaming/bridging/etc) and
>also makes vlans on top of macvtap work as before.
>
>Fixes: 6acf54f1cf (macvtap: Add support of packet capture on macvtap device)
>Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>---
> net/core/dev.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index ab9a165..54691d1 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -3642,17 +3642,6 @@ ncls:
> if (pfmemalloc && !skb_pfmemalloc_protocol(skb))
> goto drop;
>
>- if (vlan_tx_tag_present(skb)) {
>- if (pt_prev) {
>- ret = deliver_skb(skb, pt_prev, orig_dev);
>- pt_prev = NULL;
>- }
>- if (vlan_do_receive(&skb))
>- goto another_round;
>- else if (unlikely(!skb))
>- goto unlock;
>- }
>-
> rx_handler = rcu_dereference(skb->dev->rx_handler);
> if (rx_handler) {
> if (pt_prev) {
>@@ -3674,6 +3663,17 @@ ncls:
> }
> }
>
>+ if (vlan_tx_tag_present(skb)) {
>+ if (pt_prev) {
>+ ret = deliver_skb(skb, pt_prev, orig_dev);
>+ pt_prev = NULL;
>+ }
>+ if (vlan_do_receive(&skb))
>+ goto another_round;
>+ else if (unlikely(!skb))
>+ goto unlock;
>+ }
>+
nack. This will definitelly break several stacked setups.
> if (unlikely(vlan_tx_tag_present(skb))) {
> if (vlan_tx_tag_get_id(skb))
> skb->pkt_type = PACKET_OTHERHOST;
>--
>1.9.3
>
^ 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