* Re: [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT
From: David Ahern @ 2018-11-08 18:26 UTC (permalink / raw)
To: Song Liu, Peter Zijlstra
Cc: Netdev, lkml, Kernel Team, ast@kernel.org, daniel@iogearbox.net,
acme@kernel.org
In-Reply-To: <C858C862-E523-4CE8-AB39-CC9B27BE2538@fb.com>
On 11/8/18 11:04 AM, Song Liu wrote:
> On the other hand, processing BPF load/unload events synchronously should
> not introduce too much overhead for meaningful use cases. If many BPF progs
> are being loaded/unloaded within short period of time, it is not the steady
> state that profiling works care about.
but, profiling is not the only use case, and perf-record is common with
those other use cases.
I think that answers why your RFC set does not fork a thread for the bpf
events. You are focused on profiling and for already loaded programs or
for a small number of programs loaded by a specific workload started by
perf.
^ permalink raw reply
* [PATCH net-next] cxgb4: Add new T6 PCI device ids 0x608a
From: Ganesh Goudar @ 2018-11-08 8:51 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, dt, Ganesh Goudar
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
index 60df66f..bf7325f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
@@ -217,6 +217,7 @@ CH_PCI_DEVICE_ID_TABLE_DEFINE_BEGIN
CH_PCI_ID_TABLE_FENTRY(0x6087), /* Custom T6225-CR */
CH_PCI_ID_TABLE_FENTRY(0x6088), /* Custom T62100-CR */
CH_PCI_ID_TABLE_FENTRY(0x6089), /* Custom T62100-KR */
+ CH_PCI_ID_TABLE_FENTRY(0x608a), /* Custom T62100-CR */
CH_PCI_DEVICE_ID_TABLE_DEFINE_END;
#endif /* __T4_PCI_ID_TBL_H__ */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] staging: net: ipv4: tcp_vegas: fixed checks and warnings
From: David Miller @ 2018-11-08 18:34 UTC (permalink / raw)
To: suraj1998; +Cc: kuznet, yoshfuji, netdev, linux-kernel, edumazet
In-Reply-To: <1541665792-5888-1-git-send-email-suraj1998@gmail.com>
From: Suraj Singh <suraj1998@gmail.com>
Date: Thu, 8 Nov 2018 13:59:52 +0530
> Fixed checks and warnings in TCP Vegas
>
> Signed-off-by: Suraj Singh <suraj1998@gmail.com>
Once again, please explain why you are putting "staging: " into the subject
line of your TCP congestion control patches.
If you do not answer this question and fix your Subject lines, I will
really have no choice but to ignore your submissions as you are
ignoring our feedback.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 05/10] ipv6: factor out protocol delivery helper
From: Sergei Shtylyov @ 2018-11-08 9:01 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: David S. Miller, Willem de Bruijn, Steffen Klassert,
Subash Abhinov Kasiviswanathan
In-Reply-To: <12bde3e41997a124df10c1af678ace0af50e5d9e.1541588248.git.pabeni@redhat.com>
On 11/7/2018 2:38 PM, Paolo Abeni wrote:
> So that we can re-use it at the UDP level in the next patch
>
> rfc v3 -> v1:
> - add the helper declaration into the ipv6 header
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/net/ipv6.h | 2 ++
> net/ipv6/ip6_input.c | 28 ++++++++++++++++------------
> 2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 829650540780..daf80863d3a5 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
[...]
> @@ -319,28 +319,26 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
> /*
> * Deliver the packet to the host
> */
> -
> -
> -static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
> +void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
> + bool have_final)
> {
> const struct inet6_protocol *ipprot;
> struct inet6_dev *idev;
> unsigned int nhoff;
> - int nexthdr;
> bool raw;
> - bool have_final = false;
>
> /*
> * Parse extension headers
> */
>
> - rcu_read_lock();
> resubmit:
> idev = ip6_dst_idev(skb_dst(skb));
> - if (!pskb_pull(skb, skb_transport_offset(skb)))
> - goto discard;
> nhoff = IP6CB(skb)->nhoff;
> - nexthdr = skb_network_header(skb)[nhoff];
> + if (!have_final) {
Haven't you removed this variable above?
> + if (!pskb_pull(skb, skb_transport_offset(skb)))
> + goto discard;
> + nexthdr = skb_network_header(skb)[nhoff];
And this?
> + }
>
> resubmit_final:
> raw = raw6_local_deliver(skb, nexthdr);
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH bpf-next 0/2] bpf: offer maximum packet offset info
From: Jiong Wang @ 2018-11-08 9:08 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, oss-drivers, Jiong Wang
The maximum packet offset accessed by one BPF program is useful
information.
Because sometimes there could be packet split and it is possible for some
reasons (for example performance) we want to reject the BPF program if the
maximum packet size would trigger such split. Normally, MTU value is
treated as the maximum packet size, but one BPF program does not always
access the whole packet, it could only access the head portion of the data.
We could let verifier calculate the maximum packet offset ever used and
record it inside prog auxiliar information structure as a new field
"max_pkt_offset".
Jiong Wang (2):
bpf: let verifier to calculate and record max_pkt_offset
nfp: bpf: relax prog rejection through max_pkt_offset
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 9 +++++----
include/linux/bpf.h | 1 +
kernel/bpf/verifier.c | 12 ++++++++++++
3 files changed, 18 insertions(+), 4 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH bpf-next 1/2] bpf: let verifier to calculate and record max_pkt_offset
From: Jiong Wang @ 2018-11-08 9:08 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1541668123-9571-1-git-send-email-jiong.wang@netronome.com>
In check_packet_access, update max_pkt_offset after the offset has passed
__check_packet_access.
It should be safe to use u32 for max_pkt_offset as explained in code
comment.
Also, when there is tail call, the max_pkt_offset of the called program is
unknown, so conservatively set max_pkt_offset to MAX_PACKET_OFF for such
case.
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
include/linux/bpf.h | 1 +
kernel/bpf/verifier.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 33014ae..b6a296e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -293,6 +293,7 @@ struct bpf_prog_aux {
atomic_t refcnt;
u32 used_map_cnt;
u32 max_ctx_offset;
+ u32 max_pkt_offset;
u32 stack_depth;
u32 id;
u32 func_cnt;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 98fa0be..6a248d8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1452,6 +1452,17 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
verbose(env, "R%d offset is outside of the packet\n", regno);
return err;
}
+
+ /* __check_packet_access has made sure "off + size - 1" is within u16.
+ * reg->umax_value can't be bigger than MAX_PACKET_OFF which is 0xffff,
+ * otherwise find_good_pkt_pointers would have refused to set range info
+ * that __check_packet_access would have rejected this pkt access.
+ * Therefore, "off + reg->umax_value + size - 1" won't overflow u32.
+ */
+ env->prog->aux->max_pkt_offset =
+ max_t(u32, env->prog->aux->max_pkt_offset,
+ off + reg->umax_value + size - 1);
+
return err;
}
@@ -6128,6 +6139,7 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
*/
prog->cb_access = 1;
env->prog->aux->stack_depth = MAX_BPF_STACK;
+ env->prog->aux->max_pkt_offset = MAX_PACKET_OFF;
/* mark bpf_tail_call as different opcode to avoid
* conditional branch in the interpeter for every normal
--
2.7.4
^ permalink raw reply related
* [PATCH bpf-next 2/2] nfp: bpf: relax prog rejection through max_pkt_offset
From: Jiong Wang @ 2018-11-08 9:08 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, oss-drivers, Jiong Wang
In-Reply-To: <1541668123-9571-1-git-send-email-jiong.wang@netronome.com>
NFP is refusing to offload programs whenever the MTU is set to a value
larger than the max packet bytes that fits in NFP Cluster Target Memory
(CTM). However, a eBPF program doesn't always need to access the whole
packet data.
Verifier has always calculated maximum direct packet access (DPA) offset,
and kept it in max_pkt_offset inside prog auxiliar information. This patch
relax prog rejection based on max_pkt_offset.
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jiong Wang <jiong.wang@netronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/offload.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index ba8ceed..07bdc1f 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -489,14 +489,15 @@ nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog,
struct netlink_ext_ack *extack)
{
struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
- unsigned int max_mtu, max_stack, max_prog_len;
+ unsigned int fw_mtu, pkt_off, max_stack, max_prog_len;
dma_addr_t dma_addr;
void *img;
int err;
- max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
- if (max_mtu < nn->dp.netdev->mtu) {
- NL_SET_ERR_MSG_MOD(extack, "BPF offload not supported with MTU larger than HW packet split boundary");
+ fw_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
+ pkt_off = min(prog->aux->max_pkt_offset, nn->dp.netdev->mtu);
+ if (fw_mtu < pkt_off) {
+ NL_SET_ERR_MSG_MOD(extack, "BPF offload not supported with potential packet access beyond HW packet split boundary");
return -EOPNOTSUPP;
}
--
2.7.4
^ permalink raw reply related
* [PATCH iproute2-next] devlink: Add missing region option to devlink man page
From: Alex Vesker @ 2018-11-08 9:14 UTC (permalink / raw)
To: netdev, jiri; +Cc: Alex Vesker
The region field was not added to the devlink man page.
Fixes: 8b4fbf0bed8e6 ("devlink: Add support for devlink-region access")
Signed-off-by: Alex Vesker <valex@mellanox.com>
---
man/man8/devlink.8 | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index 360031f..8d527e7 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -7,7 +7,7 @@ devlink \- Devlink tool
.in +8
.ti -8
.B devlink
-.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource " } { " COMMAND " | "
+.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource | region " } { " COMMAND " | "
.BR help " }"
.sp
@@ -74,6 +74,10 @@ Turn on verbose output.
.B resource
- devlink device resource configuration.
+.TP
+.B region
+- devlink address region access
+
.SS
.I COMMAND
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT
From: Song Liu @ 2018-11-08 18:49 UTC (permalink / raw)
To: David Ahern
Cc: Peter Zijlstra, Netdev, lkml, Kernel Team, ast@kernel.org,
daniel@iogearbox.net, acme@kernel.org
In-Reply-To: <d5043aae-69b0-fd49-e82a-5a13834a3f32@gmail.com>
Hi David,
> On Nov 8, 2018, at 10:26 AM, David Ahern <dsahern@gmail.com> wrote:
>
> On 11/8/18 11:04 AM, Song Liu wrote:
>> On the other hand, processing BPF load/unload events synchronously should
>> not introduce too much overhead for meaningful use cases. If many BPF progs
>> are being loaded/unloaded within short period of time, it is not the steady
>> state that profiling works care about.
>
> but, profiling is not the only use case, and perf-record is common with
> those other use cases.
>
> I think that answers why your RFC set does not fork a thread for the bpf
> events. You are focused on profiling and for already loaded programs or
> for a small number of programs loaded by a specific workload started by
> perf.
We sure can fork a thread for the BPF event. But I guess that's not Peter's
main concern here...
Could you please point me to more information about the use cases you worry
about? I am more than happy to optimize the logic for those use cases.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH net-next 0/4] Remove VLAN_TAG_PRESENT from drivers
From: Leon Romanovsky @ 2018-11-08 18:50 UTC (permalink / raw)
To: Michał Mirosław
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Faisal Latif,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Claudiu Manoil, Shiraz Saleem
In-Reply-To: <cover.1541698641.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1100 bytes --]
On Thu, Nov 08, 2018 at 06:44:46PM +0100, Michał Mirosław wrote:
> This series removes VLAN_TAG_PRESENT use from network drivers in
> preparation to removing its special meaning.
Can you please give an extra explanation why it is removed?
Such series come out-of-blue, for people who are not following
netdev mailing list closely (drivers/infiniband/*).
Thanks
>
> Michał Mirosław (4):
> i40iw: remove use of VLAN_TAG_PRESENT
> cnic: remove use of VLAN_TAG_PRESENT
> gianfar: remove use of VLAN_TAG_PRESENT
> OVS: remove use of VLAN_TAG_PRESENT
>
> drivers/infiniband/hw/i40iw/i40iw_cm.c | 8 +++----
> drivers/net/ethernet/broadcom/cnic.c | 2 +-
> .../net/ethernet/freescale/gianfar_ethtool.c | 8 +++----
> net/openvswitch/actions.c | 13 +++++++----
> net/openvswitch/flow.c | 4 ++--
> net/openvswitch/flow.h | 2 +-
> net/openvswitch/flow_netlink.c | 22 +++++++++----------
> 7 files changed, 31 insertions(+), 28 deletions(-)
>
> --
> 2.19.1
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* [PATCH] ath10k: remove set but not used variable 'num_tdls_vifs'
From: YueHaibing @ 2018-11-08 9:32 UTC (permalink / raw)
To: Kalle Valo; +Cc: YueHaibing, ath10k, linux-wireless, netdev, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_sta_state':
drivers/net/wireless/ath/ath10k/mac.c:6238:7: warning:
variable 'num_tdls_vifs' set but not used [-Wunused-but-set-variable]
'num_tdls_vifs' not used any more after
9a993cc1ea95 ("ath10k: fix the logic of limiting tdls peer counts")
Also, remove the single called function ath10k_mac_tdls_vifs_count
and ath10k_mac_tdls_vifs_count_iter.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801..6006f7a 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5754,30 +5754,6 @@ static int ath10k_mac_tdls_vif_stations_count(struct ieee80211_hw *hw,
return data.num_tdls_stations;
}
-static void ath10k_mac_tdls_vifs_count_iter(void *data, u8 *mac,
- struct ieee80211_vif *vif)
-{
- struct ath10k_vif *arvif = (void *)vif->drv_priv;
- int *num_tdls_vifs = data;
-
- if (vif->type != NL80211_IFTYPE_STATION)
- return;
-
- if (ath10k_mac_tdls_vif_stations_count(arvif->ar->hw, vif) > 0)
- (*num_tdls_vifs)++;
-}
-
-static int ath10k_mac_tdls_vifs_count(struct ieee80211_hw *hw)
-{
- int num_tdls_vifs = 0;
-
- ieee80211_iterate_active_interfaces_atomic(hw,
- IEEE80211_IFACE_ITER_NORMAL,
- ath10k_mac_tdls_vifs_count_iter,
- &num_tdls_vifs);
- return num_tdls_vifs;
-}
-
static int ath10k_hw_scan(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_scan_request *hw_req)
@@ -6285,7 +6261,6 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
*/
enum wmi_peer_type peer_type = WMI_PEER_TYPE_DEFAULT;
u32 num_tdls_stations;
- u32 num_tdls_vifs;
ath10k_dbg(ar, ATH10K_DBG_MAC,
"mac vdev %d peer create %pM (new sta) sta %d / %d peer %d / %d\n",
@@ -6301,7 +6276,6 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
}
num_tdls_stations = ath10k_mac_tdls_vif_stations_count(hw, vif);
- num_tdls_vifs = ath10k_mac_tdls_vifs_count(hw);
if (sta->tdls) {
if (num_tdls_stations >= ar->max_num_tdls_vdevs) {
^ permalink raw reply related
* Re: [PATCH iproute2] bridge: fdb: remove redundant dev string in show output
From: Phil Sutter @ 2018-11-08 9:35 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: stephen, netdev, nikolay, dsahern
In-Reply-To: <1541632449-9993-1-git-send-email-roopa@cumulusnetworks.com>
Hi Roopa,
On Wed, Nov 07, 2018 at 03:14:09PM -0800, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> After commit 4abb8c723a64 ("bridge: fdb: Fix for missing
> keywords in non-JSON output"), I am seeing a double print for dev
> in bridge fdb show. eg:
> "44:38:39:00:6a:82 dev dev bridge vlan 1 master bridge permanent"
>
> this patch removes the redundant print.
>
> Fixes: 4abb8c723a64 ("bridge: fdb: Fix for missing keywords in non-JSON output")
Oh, stupid mistake. :(
Thanks for the fix!
> CC: Phil Sutter <phil@nwl.cc>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Acked-by: Phil Sutter <phil@nwl.cc>
^ permalink raw reply
* Re: [PATCH] net: phy: leds: Don't make our own link speed names
From: Kyle Roeschley @ 2018-11-08 19:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, David S . Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <fa2751d5-a15c-6414-9b80-15f54fd9f135@gmail.com>
On Thu, Nov 08, 2018 at 09:59:53AM -0800, Florian Fainelli wrote:
> On 11/8/18 9:51 AM, Kyle Roeschley wrote:
> > The phy core provides a handy phy_speed_to_str() helper, so use that
> > instead of doing our own formatting of the different known link speeds.
>
> In case the speed is not supported, phy_speed_to_str() would return
> "Unsupported (update phy-core.c)" which is bigger (by 21 characters)
> than name_suffix.
>
> If you bumped name_suffix/PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE to 11
> characters, that would be just large enough to accommodate for the
> "Unsupported" part of the string and that might be an acceptable
> solution in between.
>
Thanks for catching that, I'll send a v2.
--
Kyle Roeschley
Software Engineer
National Instruments
^ permalink raw reply
* Re: [PATCH net-next 05/10] ipv6: factor out protocol delivery helper
From: Paolo Abeni @ 2018-11-08 10:13 UTC (permalink / raw)
To: Sergei Shtylyov, netdev
Cc: David S. Miller, Willem de Bruijn, Steffen Klassert,
Subash Abhinov Kasiviswanathan
In-Reply-To: <34872c3b-eba8-d7eb-22da-ee74b52b0610@cogentembedded.com>
Hi,
On Thu, 2018-11-08 at 12:01 +0300, Sergei Shtylyov wrote:
> On 11/7/2018 2:38 PM, Paolo Abeni wrote:
>
> > So that we can re-use it at the UDP level in the next patch
> >
> > rfc v3 -> v1:
> > - add the helper declaration into the ipv6 header
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > include/net/ipv6.h | 2 ++
> > net/ipv6/ip6_input.c | 28 ++++++++++++++++------------
> > 2 files changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> > index 829650540780..daf80863d3a5 100644
> > --- a/include/net/ipv6.h
> > +++ b/include/net/ipv6.h
>
> [...]
> > @@ -319,28 +319,26 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
> > /*
> > * Deliver the packet to the host
> > */
> > -
> > -
> > -static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
> > +void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
> > + bool have_final)
> > {
> > const struct inet6_protocol *ipprot;
> > struct inet6_dev *idev;
> > unsigned int nhoff;
> > - int nexthdr;
> > bool raw;
> > - bool have_final = false;
> >
> > /*
> > * Parse extension headers
> > */
> >
> > - rcu_read_lock();
> > resubmit:
> > idev = ip6_dst_idev(skb_dst(skb));
> > - if (!pskb_pull(skb, skb_transport_offset(skb)))
> > - goto discard;
> > nhoff = IP6CB(skb)->nhoff;
> > - nexthdr = skb_network_header(skb)[nhoff];
> > + if (!have_final) {
>
> Haven't you removed this variable above?
>
> > + if (!pskb_pull(skb, skb_transport_offset(skb)))
> > + goto discard;
> > + nexthdr = skb_network_header(skb)[nhoff];
>
> And this?
Thanks for reviewing.
Both local variables are now function arguments (see the function
signature, far above in this chunk).
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH net-next 05/10] ipv6: factor out protocol delivery helper
From: Sergei Shtylyov @ 2018-11-08 10:29 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: David S. Miller, Willem de Bruijn, Steffen Klassert,
Subash Abhinov Kasiviswanathan
In-Reply-To: <0c668ae869c111bbfce9c9ba04e5c6ab2ce3c10a.camel@redhat.com>
On 11/8/2018 1:13 PM, Paolo Abeni wrote:
>>> So that we can re-use it at the UDP level in the next patch
>>>
>>> rfc v3 -> v1:
>>> - add the helper declaration into the ipv6 header
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>> include/net/ipv6.h | 2 ++
>>> net/ipv6/ip6_input.c | 28 ++++++++++++++++------------
>>> 2 files changed, 18 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
>>> index 829650540780..daf80863d3a5 100644
>>> --- a/include/net/ipv6.h
>>> +++ b/include/net/ipv6.h
>>
>> [...]
>>> @@ -319,28 +319,26 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
>>> /*
>>> * Deliver the packet to the host
>>> */
>>> -
>>> -
>>> -static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
>>> +void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
>>> + bool have_final)
>>> {
>>> const struct inet6_protocol *ipprot;
>>> struct inet6_dev *idev;
>>> unsigned int nhoff;
>>> - int nexthdr;
>>> bool raw;
>>> - bool have_final = false;
>>>
>>> /*
>>> * Parse extension headers
>>> */
>>>
>>> - rcu_read_lock();
>>> resubmit:
>>> idev = ip6_dst_idev(skb_dst(skb));
>>> - if (!pskb_pull(skb, skb_transport_offset(skb)))
>>> - goto discard;
>>> nhoff = IP6CB(skb)->nhoff;
>>> - nexthdr = skb_network_header(skb)[nhoff];
>>> + if (!have_final) {
>>
>> Haven't you removed this variable above?
>>
>>> + if (!pskb_pull(skb, skb_transport_offset(skb)))
>>> + goto discard;
>>> + nexthdr = skb_network_header(skb)[nhoff];
>>
>> And this?
>
> Thanks for reviewing.
>
> Both local variables are now function arguments (see the function
> signature, far above in this chunk).
Oops, my bad... I'm sorry. :-)
> Cheers,
>
> Paolo
MBR, Sergei
^ permalink raw reply
* [PATCH v2 net-next 0/4] net: ethernet: ti: cpsw: fix vlan mcast
From: Ivan Khoronzhuk @ 2018-11-08 20:27 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, alexander.h.duyck, bjorn,
Ivan Khoronzhuk
The cpsw holds separate mcast entires for vlan entries. At this moment
driver adds only not vlan mcast addresses, omitting vlan/mcast entries.
As result mcast for vlans doesn't work. It can be fixed by adding same
mcast entries for every created vlan, but this patchseries uses more
sophisticated way and allows to create mcast entries only for vlans
that really require it. Generic functions from this series can be
reused for fixing vlan and macvlan unicast.
Simple example of ALE table before and after this series, having same
mcast entries as for vlan 100 as for real device (reserved vlan 2),
and one mcast address only for vlan 100 - 01:1b:19:00:00:00.
<---- Before this patchset ---->
vlan , vid = 2, untag_force = 0x5, reg_mcast = 0x5, mem_list = 0x5
mcast, vid = 2, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
ucast, vid = 2, addr = 74:da:ea:47:7d:9d, persistant, port_num = 0x0
vlan , vid = 0, untag_force = 0x7, reg_mcast = 0x0, mem_list = 0x7
mcast, vid = 2, addr = 33:33:00:00:00:01, port_mask = 0x1
mcast, vid = 2, addr = 01:00:5e:00:00:01, port_mask = 0x1
vlan , vid = 1, untag_force = 0x3, reg_mcast = 0x3, mem_list = 0x3
mcast, vid = 1, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
ucast, vid = 1, addr = 74:da:ea:47:7d:9c, persistant, port_num = 0x0
mcast, vid = 1, addr = 33:33:00:00:00:01, port_mask = 0x1
mcast, vid = 1, addr = 01:00:5e:00:00:01, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:00, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:03, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:0e, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:00, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:03, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:0e, port_mask = 0x1
mcast, vid = 2, addr = 33:33:ff:47:7d:9d, port_mask = 0x1
mcast, vid = 2, addr = 33:33:00:00:00:fb, port_mask = 0x1
mcast, vid = 2, addr = 33:33:00:01:00:03, port_mask = 0x1
mcast, vid = 1, addr = 33:33:ff:47:7d:9c, port_mask = 0x1
mcast, vid = 1, addr = 33:33:00:00:00:fb, port_mask = 0x1
mcast, vid = 1, addr = 33:33:00:01:00:03, port_mask = 0x1
mcast, vid = 1, addr = 01:00:5e:00:00:fb, port_mask = 0x1
mcast, vid = 1, addr = 01:00:5e:00:00:fc, port_mask = 0x1
vlan , vid = 100, untag_force = 0x0, reg_mcast = 0x5, mem_list = 0x5
ucast, vid = 100, addr = 74:da:ea:47:7d:9d, persistant, port_num = 0x0
mcast, vid = 100, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
mcast, vid = 2, addr = 01:1b:19:00:00:00, port_mask = 0x1
^^^
Here mcast entry (ptpl2), has to be added only for vlan 100
but added for reserved vlan 2...that's not enough.
<---- After this patchset ---->
vlan , vid = 2, untag_force = 0x5, reg_mcast = 0x5, mem_list = 0x5
mcast, vid = 2, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
ucast, vid = 2, addr = 74:da:ea:47:7d:9d, persistant, port_num = 0x0
vlan , vid = 0, untag_force = 0x7, reg_mcast = 0x0, mem_list = 0x7
mcast, vid = 2, addr = 33:33:00:00:00:01, port_mask = 0x1
mcast, vid = 2, addr = 01:00:5e:00:00:01, port_mask = 0x1
vlan , vid = 1, untag_force = 0x3, reg_mcast = 0x3, mem_list = 0x3
mcast, vid = 1, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
ucast, vid = 1, addr = 74:da:ea:47:7d:9c, persistant, port_num = 0x0
mcast, vid = 1, addr = 33:33:00:00:00:01, port_mask = 0x1
mcast, vid = 1, addr = 01:00:5e:00:00:01, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:00, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:03, port_mask = 0x1
mcast, vid = 2, addr = 01:80:c2:00:00:0e, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:00, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:03, port_mask = 0x1
mcast, vid = 1, addr = 01:80:c2:00:00:0e, port_mask = 0x1
mcast, vid = 2, addr = 33:33:ff:47:7d:9d, port_mask = 0x1
mcast, vid = 1, addr = 33:33:ff:47:7d:9c, port_mask = 0x1
mcast, vid = 2, addr = 33:33:00:00:00:fb, port_mask = 0x1
mcast, vid = 2, addr = 33:33:00:01:00:03, port_mask = 0x1
mcast, vid = 1, addr = 33:33:00:00:00:fb, port_mask = 0x1
mcast, vid = 1, addr = 33:33:00:01:00:03, port_mask = 0x1
vlan , vid = 100, untag_force = 0x0, reg_mcast = 0x5, mem_list = 0x5
ucast, vid = 100, addr = 74:da:ea:47:7d:9d, persistant, port_num = 0x0
mcast, vid = 100, addr = ff:ff:ff:ff:ff:ff, port_mask = 0x1
mcast, vid = 100, addr = 33:33:00:00:00:01, port_mask = 0x1
mcast, vid = 100, addr = 01:00:5e:00:00:01, port_mask = 0x1
mcast, vid = 100, addr = 33:33:ff:47:7d:9d, port_mask = 0x1
mcast, vid = 100, addr = 01:80:c2:00:00:00, port_mask = 0x1
mcast, vid = 100, addr = 01:80:c2:00:00:03, port_mask = 0x1
mcast, vid = 100, addr = 01:80:c2:00:00:0e, port_mask = 0x1
mcast, vid = 100, addr = 33:33:00:00:00:fb, port_mask = 0x1
mcast, vid = 100, addr = 33:33:00:01:00:03, port_mask = 0x1
mcast, vid = 100, addr = 01:1b:19:00:00:00, port_mask = 0x1
^^^
Here mcast entry (ptpl2), is added only for vlan 100
as it should be.
Based on net-next/master
v2..v1:
net: ethernet: ti: cpsw: fix vlan mcast
- removed limit for legacy switch cpsw mode
Ivan Khoronzhuk (4):
net: core: dev_addr_lists: add auxiliary func to handle reference
address updates
net: 8021q: vlan_core: allow use list of vlans for real device
net: ethernet: ti: cpsw: fix vlan mcast
net: ethernet: ti: cpsw: fix vlan configuration while down/up
drivers/net/ethernet/ti/cpsw.c | 186 +++++++++++++++++++++++++++------
include/linux/if_vlan.h | 11 ++
include/linux/netdevice.h | 10 ++
net/8021q/vlan_core.c | 27 +++++
net/core/dev_addr_lists.c | 97 +++++++++++++++++
5 files changed, 300 insertions(+), 31 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2 net-next 2/4] net: 8021q: vlan_core: allow use list of vlans for real device
From: Ivan Khoronzhuk @ 2018-11-08 20:27 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, alexander.h.duyck, bjorn,
Ivan Khoronzhuk
In-Reply-To: <20181108202757.30110-1-ivan.khoronzhuk@linaro.org>
It's redundancy for the drivers to hold the list of vlans when
absolutely the same list exists in vlan core. In most cases it's
needed only to traverse the vlan devices, their vids and sync some
settings with h/w, so add API to simplify this.
At least some of these drivers also can benefit:
grep "for_each.*vid" -r drivers/net/ethernet/
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:
drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c:
drivers/net/ethernet/qlogic/qlge/qlge_main.c:
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c:
drivers/net/ethernet/via/via-rhine.c:
drivers/net/ethernet/via/via-velocity.c:
drivers/net/ethernet/intel/igb/igb_main.c:
drivers/net/ethernet/intel/ice/ice_main.c:
drivers/net/ethernet/intel/e1000/e1000_main.c:
drivers/net/ethernet/intel/i40e/i40e_main.c:
drivers/net/ethernet/intel/e1000e/netdev.c:
drivers/net/ethernet/intel/igbvf/netdev.c:
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:
drivers/net/ethernet/intel/ixgb/ixgb_main.c:
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:
drivers/net/ethernet/amd/xgbe/xgbe-dev.c:
drivers/net/ethernet/emulex/benet/be_main.c:
drivers/net/ethernet/neterion/vxge/vxge-main.c:
drivers/net/ethernet/adaptec/starfire.c:
drivers/net/ethernet/brocade/bna/bnad.c:
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
include/linux/if_vlan.h | 11 +++++++++++
net/8021q/vlan_core.c | 27 +++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 03b08ffded07..1be5230921b5 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -133,6 +133,9 @@ struct vlan_pcpu_stats {
extern struct net_device *__vlan_find_dev_deep_rcu(struct net_device *real_dev,
__be16 vlan_proto, u16 vlan_id);
+extern int vlan_for_each(struct net_device *dev,
+ int (*action)(struct net_device *dev, int vid,
+ void *arg), void *arg);
extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
extern u16 vlan_dev_vlan_id(const struct net_device *dev);
extern __be16 vlan_dev_vlan_proto(const struct net_device *dev);
@@ -236,6 +239,14 @@ __vlan_find_dev_deep_rcu(struct net_device *real_dev,
return NULL;
}
+static inline int
+vlan_for_each(struct net_device *dev,
+ int (*action)(struct net_device *dev, int vid, void *arg),
+ void *arg)
+{
+ return 0;
+}
+
static inline struct net_device *vlan_dev_real_dev(const struct net_device *dev)
{
BUG();
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4f60e86f4b8d..6308b5427a66 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -223,6 +223,33 @@ static int vlan_kill_rx_filter_info(struct net_device *dev, __be16 proto, u16 vi
return -ENODEV;
}
+int vlan_for_each(struct net_device *dev,
+ int (*action)(struct net_device *dev, int vid, void *arg),
+ void *arg)
+{
+ struct vlan_vid_info *vid_info;
+ struct vlan_info *vlan_info;
+ struct net_device *vdev;
+ int ret;
+
+ ASSERT_RTNL();
+
+ vlan_info = rtnl_dereference(dev->vlan_info);
+ if (!vlan_info)
+ return 0;
+
+ list_for_each_entry(vid_info, &vlan_info->vid_list, list) {
+ vdev = vlan_group_get_device(&vlan_info->grp, vid_info->proto,
+ vid_info->vid);
+ ret = action(vdev, vid_info->vid, arg);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(vlan_for_each);
+
int vlan_filter_push_vids(struct vlan_info *vlan_info, __be16 proto)
{
struct net_device *real_dev = vlan_info->real_dev;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 3/4] net: ethernet: ti: cpsw: fix vlan mcast
From: Ivan Khoronzhuk @ 2018-11-08 20:27 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, alexander.h.duyck, bjorn,
Ivan Khoronzhuk
In-Reply-To: <20181108202757.30110-1-ivan.khoronzhuk@linaro.org>
At this moment, mcast addresses are added for real device only
(reserved vlans for dual-emac mode), even if a mcast address was added
for some vlan only, thus ALE doesn't have corresponding vlan mcast
entries after vlan socket joined multicast group. So ALE drops vlan
frames with mcast addresses intended for vlans and potentially can
receive mcast frames for base ndev. That's not correct. So, fix it by
creating only vlan/mcast entries as requested. Patch doesn't use any
additional lists and is based on device mc address list and cpsw ALE
table entries.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw.c | 169 +++++++++++++++++++++++++++------
1 file changed, 138 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 500f7ed8c58c..0b18634d336c 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -570,21 +570,6 @@ static inline int cpsw_get_slave_port(u32 slave_num)
return slave_num + 1;
}
-static void cpsw_add_mcast(struct cpsw_priv *priv, const u8 *addr)
-{
- struct cpsw_common *cpsw = priv->cpsw;
-
- if (cpsw->data.dual_emac) {
- struct cpsw_slave *slave = cpsw->slaves + priv->emac_port;
-
- cpsw_ale_add_mcast(cpsw->ale, addr, ALE_PORT_HOST,
- ALE_VLAN, slave->port_vlan, 0);
- return;
- }
-
- cpsw_ale_add_mcast(cpsw->ale, addr, ALE_ALL_PORTS, 0, 0, 0);
-}
-
static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
{
struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
@@ -640,7 +625,7 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
/* Clear all mcast from ALE */
cpsw_ale_flush_multicast(ale, ALE_ALL_PORTS, -1);
- __dev_mc_unsync(ndev, NULL);
+ __hw_addr_ref_unsync_dev(&ndev->mc, ndev, NULL);
/* Flood All Unicast Packets to Host port */
cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 1);
@@ -661,29 +646,148 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
}
}
-static int cpsw_add_mc_addr(struct net_device *ndev, const u8 *addr)
+struct addr_sync_ctx {
+ struct net_device *ndev;
+ const u8 *addr; /* address to be synched */
+ int consumed; /* number of address instances */
+ int flush; /* flush flag */
+};
+
+/**
+ * cpsw_set_mc - adds multicast entry to the table if it's not added or deletes
+ * if it's not deleted
+ * @ndev: device to sync
+ * @addr: address to be added or deleted
+ * @vid: vlan id, if vid < 0 set/unset address for real device
+ * @add: add address if the flag is set or remove otherwise
+ */
+static int cpsw_set_mc(struct net_device *ndev, const u8 *addr,
+ int vid, int add)
{
struct cpsw_priv *priv = netdev_priv(ndev);
+ struct cpsw_common *cpsw = priv->cpsw;
+ int mask, flags, ret;
+
+ if (vid < 0) {
+ if (cpsw->data.dual_emac)
+ vid = cpsw->slaves[priv->emac_port].port_vlan;
+ else
+ vid = 0;
+ }
+
+ mask = cpsw->data.dual_emac ? ALE_PORT_HOST : ALE_ALL_PORTS;
+ flags = vid ? ALE_VLAN : 0;
+
+ if (add)
+ ret = cpsw_ale_add_mcast(cpsw->ale, addr, mask, flags, vid, 0);
+ else
+ ret = cpsw_ale_del_mcast(cpsw->ale, addr, 0, flags, vid);
+
+ return ret;
+}
+
+static int cpsw_update_vlan_mc(struct net_device *vdev, int vid, void *ctx)
+{
+ struct addr_sync_ctx *sync_ctx = ctx;
+ struct netdev_hw_addr *ha;
+ int found = 0, ret = 0;
+
+ if (!vdev || !(vdev->flags & IFF_UP))
+ return 0;
+
+ /* vlan address is relevant if its sync_cnt != 0 */
+ netdev_for_each_mc_addr(ha, vdev) {
+ if (ether_addr_equal(ha->addr, sync_ctx->addr)) {
+ found = ha->sync_cnt;
+ break;
+ }
+ }
+
+ if (found)
+ sync_ctx->consumed++;
+
+ if (sync_ctx->flush) {
+ if (!found)
+ cpsw_set_mc(sync_ctx->ndev, sync_ctx->addr, vid, 0);
+ return 0;
+ }
+
+ if (found)
+ ret = cpsw_set_mc(sync_ctx->ndev, sync_ctx->addr, vid, 1);
+
+ return ret;
+}
+
+static int cpsw_add_mc_addr(struct net_device *ndev, const u8 *addr, int num)
+{
+ struct addr_sync_ctx sync_ctx;
+ int ret;
+
+ sync_ctx.consumed = 0;
+ sync_ctx.addr = addr;
+ sync_ctx.ndev = ndev;
+ sync_ctx.flush = 0;
+
+ ret = vlan_for_each(ndev, cpsw_update_vlan_mc, &sync_ctx);
+ if (sync_ctx.consumed < num && !ret)
+ ret = cpsw_set_mc(ndev, addr, -1, 1);
+
+ return ret;
+}
+
+static int cpsw_del_mc_addr(struct net_device *ndev, const u8 *addr, int num)
+{
+ struct addr_sync_ctx sync_ctx;
+
+ sync_ctx.consumed = 0;
+ sync_ctx.addr = addr;
+ sync_ctx.ndev = ndev;
+ sync_ctx.flush = 1;
+
+ vlan_for_each(ndev, cpsw_update_vlan_mc, &sync_ctx);
+ if (sync_ctx.consumed == num)
+ cpsw_set_mc(ndev, addr, -1, 0);
- cpsw_add_mcast(priv, addr);
return 0;
}
-static int cpsw_del_mc_addr(struct net_device *ndev, const u8 *addr)
+static int cpsw_purge_vlan_mc(struct net_device *vdev, int vid, void *ctx)
{
- struct cpsw_priv *priv = netdev_priv(ndev);
- struct cpsw_common *cpsw = priv->cpsw;
- int vid, flags;
+ struct addr_sync_ctx *sync_ctx = ctx;
+ struct netdev_hw_addr *ha;
+ int found = 0;
- if (cpsw->data.dual_emac) {
- vid = cpsw->slaves[priv->emac_port].port_vlan;
- flags = ALE_VLAN;
- } else {
- vid = 0;
- flags = 0;
+ if (!vdev || !(vdev->flags & IFF_UP))
+ return 0;
+
+ /* vlan address is relevant if its sync_cnt != 0 */
+ netdev_for_each_mc_addr(ha, vdev) {
+ if (ether_addr_equal(ha->addr, sync_ctx->addr)) {
+ found = ha->sync_cnt;
+ break;
+ }
}
- cpsw_ale_del_mcast(cpsw->ale, addr, 0, flags, vid);
+ if (!found)
+ return 0;
+
+ sync_ctx->consumed++;
+ cpsw_set_mc(sync_ctx->ndev, sync_ctx->addr, vid, 0);
+ return 0;
+}
+
+static int cpsw_purge_all_mc(struct net_device *ndev, const u8 *addr, int num)
+{
+ struct addr_sync_ctx sync_ctx;
+
+ sync_ctx.addr = addr;
+ sync_ctx.ndev = ndev;
+ sync_ctx.consumed = 0;
+
+ vlan_for_each(ndev, cpsw_purge_vlan_mc, &sync_ctx);
+ if (sync_ctx.consumed < num)
+ cpsw_set_mc(ndev, addr, -1, 0);
+
return 0;
}
@@ -704,7 +808,9 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
/* Restore allmulti on vlans if necessary */
cpsw_ale_set_allmulti(cpsw->ale, ndev->flags & IFF_ALLMULTI);
- __dev_mc_sync(ndev, cpsw_add_mc_addr, cpsw_del_mc_addr);
+ /* add/remove mcast address either for real netdev or for vlan */
+ __hw_addr_ref_sync_dev(&ndev->mc, ndev, cpsw_add_mc_addr,
+ cpsw_del_mc_addr);
}
static void cpsw_intr_enable(struct cpsw_common *cpsw)
@@ -1964,7 +2070,7 @@ static int cpsw_ndo_stop(struct net_device *ndev)
struct cpsw_common *cpsw = priv->cpsw;
cpsw_info(priv, ifdown, "shutting down cpsw device\n");
- __dev_mc_unsync(priv->ndev, cpsw_del_mc_addr);
+ __hw_addr_ref_unsync_dev(&ndev->mc, ndev, cpsw_purge_all_mc);
netif_tx_stop_all_queues(priv->ndev);
netif_carrier_off(priv->ndev);
@@ -2415,6 +2521,7 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
HOST_PORT_NUM, ALE_VLAN, vid);
ret |= cpsw_ale_del_mcast(cpsw->ale, priv->ndev->broadcast,
0, ALE_VLAN, vid);
+ ret |= cpsw_ale_flush_multicast(cpsw->ale, 0, vid);
err:
pm_runtime_put(cpsw->dev);
return ret;
--
2.17.1
^ permalink raw reply related
* Re: [net-next 12/12] igc: Clean up code
From: Joe Perches @ 2018-11-08 11:00 UTC (permalink / raw)
To: Jeff Kirsher, davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann
In-Reply-To: <20181107224830.9737-13-jeffrey.t.kirsher@intel.com>
On Wed, 2018-11-07 at 14:48 -0800, Jeff Kirsher wrote:
> From: Sasha Neftin <sasha.neftin@intel.com>
>
> Address few community comments.
> Remove unused code, will be added per demand.
> Remove blank lines and unneeded includes.
>
> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/igc/igc.h | 9 ---------
> drivers/net/ethernet/intel/igc/igc_main.c | 15 ---------------
> 2 files changed, 24 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
[]
> #define IGC_ERR(args...) pr_err("igc: " args)
This is used just once and should probably be removed.
maybe:
---
drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 9d85707e8a81..b58542b20623 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3551,7 +3551,7 @@ static int igc_probe(struct pci_dev *pdev,
err = dma_set_coherent_mask(&pdev->dev,
DMA_BIT_MASK(32));
if (err) {
- IGC_ERR("Wrong DMA configuration, aborting\n");
+ dev_err(&pdev->dev, "igc: Wrong DMA configuration, aborting\n");
goto err_dma;
}
}
^ permalink raw reply related
* Re: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector
From: Quentin Monnet @ 2018-11-08 11:16 UTC (permalink / raw)
To: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski
Cc: guro, jiong.wang, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181108053957.205681-5-sdf@google.com>
Hi Stanislav, thanks for the changes! More comments below.
2018-11-07 21:39 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
> This commit adds support for loading/attaching/detaching flow
> dissector program. The structure of the flow dissector program is
> assumed to be the same as in the selftests:
>
> * flow_dissector section with the main entry point
> * a bunch of tail call progs
> * a jmp_table map that is populated with the tail call progs
>
> When `bpftool load` is called with a flow_dissector prog (i.e. when the
> first section is flow_dissector of 'type flow_dissector' argument is
> passed), we load and pin all the programs/maps. User is responsible to
> construct the jump table for the tail calls.
>
> The last argument of `bpftool attach` is made optional for this use
> case.
>
> Example:
> bpftool prog load tools/testing/selftests/bpf/bpf_flow.o \
> /sys/fs/bpf/flow type flow_dissector
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 0 0 0 0 \
> value pinned /sys/fs/bpf/flow/IP
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 1 0 0 0 \
> value pinned /sys/fs/bpf/flow/IPV6
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 2 0 0 0 \
> value pinned /sys/fs/bpf/flow/IPV6OP
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 3 0 0 0 \
> value pinned /sys/fs/bpf/flow/IPV6FR
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 4 0 0 0 \
> value pinned /sys/fs/bpf/flow/MPLS
>
> bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
> key 5 0 0 0 \
> value pinned /sys/fs/bpf/flow/VLAN
>
> bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector flow_dissector
>
> Tested by using the above lines to load the prog in
> the test_flow_dissector.sh selftest.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> .../bpftool/Documentation/bpftool-prog.rst | 36 ++++--
> tools/bpf/bpftool/bash-completion/bpftool | 6 +-
> tools/bpf/bpftool/common.c | 30 ++---
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 112 +++++++++++++-----
> 5 files changed, 126 insertions(+), 59 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ac4e904b10fb..0374634c3087 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -15,7 +15,8 @@ SYNOPSIS
> *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
>
> *COMMANDS* :=
> - { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
> + { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
> + | **loadall** | **help** }
>
> MAP COMMANDS
> =============
> @@ -24,9 +25,9 @@ MAP COMMANDS
> | **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
> | **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
> | **bpftool** **prog pin** *PROG* *FILE*
> -| **bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> -| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> -| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> +| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> +| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> | **bpftool** **prog help**
> |
> | *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> @@ -39,7 +40,9 @@ MAP COMMANDS
> | **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
> | **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
> | }
> -| *ATTACH_TYPE* := { **msg_verdict** | **skb_verdict** | **skb_parse** }
> +| *ATTACH_TYPE* := {
> +| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
> +| }
>
>
> DESCRIPTION
> @@ -79,8 +82,11 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> Load bpf program from binary *OBJ* and pin as *FILE*.
> + **bpftool prog load** will pin only the first bpf program
> + from the *OBJ*, **bpftool prog loadall** will pin all maps
> + and programs from the *OBJ*.
This could be improved regarding maps: with "bpftool prog load" I think
we also load and pin all maps, but your description implies this is only
the case with "loadall"
> **type** is optional, if not specified program type will be
> inferred from section names.
> By default bpftool will create new maps as declared in the ELF
> @@ -97,13 +103,17 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **bpftool prog attach** *PROG* *ATTACH_TYPE* *MAP*
> - Attach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
> - to the map *MAP*.
> -
> - **bpftool prog detach** *PROG* *ATTACH_TYPE* *MAP*
> - Detach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
> - from the map *MAP*.
> + **bpftool prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> + Attach bpf program *PROG* (with type specified by
> + *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
> + parameter, with the exception of *flow_dissector* which is
> + attached to current networking name space.
> +
> + **bpftool prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
> + Detach bpf program *PROG* (with type specified by
> + *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
> + parameter, with the exception of *flow_dissector* which is
> + detached from the current networking name space.
While at it could you please fix those two paragraphs to use tabs for
indentation, as the rest of the doc? Thanks!
>
> **bpftool prog help**
> Print short help message.
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 3f78e6404589..ad0fc919f7ec 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -243,7 +243,7 @@ _bpftool()
> # Completion depends on object and command in use
> case $object in
> prog)
> - if [[ $command != "load" ]]; then
> + if [[ $command != "load" && $command != "loadall" ]]; then
> case $prev in
> id)
> _bpftool_get_prog_ids
> @@ -299,7 +299,7 @@ _bpftool()
> fi
>
> if [[ ${#words[@]} == 6 ]]; then
> - COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse" -- "$cur" ) )
> + COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse flow_dissector" -- "$cur" ) )
> return 0
> fi
>
> @@ -309,7 +309,7 @@ _bpftool()
> fi
> return 0
> ;;
> - load)
> + load|loadall)
> local obj
>
> if [[ ${#words[@]} -lt 6 ]]; then
You also want to update completion for the program types, at line 341 or
so. Feel free to split that list on several lines, by the way :).
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 25af85304ebe..f671a921dec5 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -169,34 +169,24 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
> return fd;
> }
>
> -int do_pin_fd(int fd, const char *name)
> +int mount_bpffs_for_pin(const char *name)
> {
> char err_str[ERR_MAX_LEN];
> char *file;
> char *dir;
> int err = 0;
>
> - err = bpf_obj_pin(fd, name);
> - if (!err)
> - goto out;
> -
> file = malloc(strlen(name) + 1);
> strcpy(file, name);
> dir = dirname(file);
>
> - if (errno != EPERM || is_bpffs(dir)) {
> - p_err("can't pin the object (%s): %s", name, strerror(errno));
> + if (is_bpffs(dir)) {
> + /* nothing to do if already mounted */
> goto out_free;
> }
Nitpick: unnecessary brackets.
>
> - /* Attempt to mount bpffs, then retry pinning. */
> err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
> - if (!err) {
> - err = bpf_obj_pin(fd, name);
> - if (err)
> - p_err("can't pin the object (%s): %s", name,
> - strerror(errno));
> - } else {
> + if (err) {
> err_str[ERR_MAX_LEN - 1] = '\0';
> p_err("can't mount BPF file system to pin the object (%s): %s",
> name, err_str);
> @@ -204,10 +194,20 @@ int do_pin_fd(int fd, const char *name)
>
> out_free:
> free(file);
> -out:
> return err;
> }
>
> +int do_pin_fd(int fd, const char *name)
> +{
> + int err;
> +
> + err = mount_bpffs_for_pin(name);
> + if (err)
> + return err;
> +
> + return bpf_obj_pin(fd, name);
> +}
> +
> int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
> {
> unsigned int id;
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 28322ace2856..1383824c9baf 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -129,6 +129,7 @@ const char *get_fd_type_name(enum bpf_obj_type type);
> char *get_fdinfo(int fd, const char *key);
> int open_obj_pinned(char *path);
> int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
> +int mount_bpffs_for_pin(const char *name);
> int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
> int do_pin_fd(int fd, const char *name);
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee282409..a4346dd673b1 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
> [BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
> [BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
> [BPF_SK_MSG_VERDICT] = "msg_verdict",
> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
> [__MAX_BPF_ATTACH_TYPE] = NULL,
> };
>
> @@ -724,10 +725,11 @@ int map_replace_compar(const void *p1, const void *p2)
> static int do_attach(int argc, char **argv)
> {
> enum bpf_attach_type attach_type;
> - int err, mapfd, progfd;
> + int err, progfd;
> + int mapfd = 0;
>
> - if (!REQ_ARGS(5)) {
> - p_err("too few parameters for map attach");
> + if (!REQ_ARGS(3)) {
> + p_err("too few parameters for attach");
> return -EINVAL;
> }
>
> @@ -740,11 +742,17 @@ static int do_attach(int argc, char **argv)
> p_err("invalid attach type");
> return -EINVAL;
> }
> - NEXT_ARG();
> + if (attach_type != BPF_FLOW_DISSECTOR) {
> + NEXT_ARG();
> + if (!REQ_ARGS(2)) {
> + p_err("too few parameters for map attach");
> + return -EINVAL;
> + }
>
> - mapfd = map_parse_fd(&argc, &argv);
> - if (mapfd < 0)
> - return mapfd;
> + mapfd = map_parse_fd(&argc, &argv);
> + if (mapfd < 0)
> + return mapfd;
> + }
>
> err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
> if (err) {
> @@ -760,10 +768,11 @@ static int do_attach(int argc, char **argv)
> static int do_detach(int argc, char **argv)
> {
> enum bpf_attach_type attach_type;
> - int err, mapfd, progfd;
> + int err, progfd;
> + int mapfd = 0;
>
> - if (!REQ_ARGS(5)) {
> - p_err("too few parameters for map detach");
> + if (!REQ_ARGS(3)) {
> + p_err("too few parameters for detach");
> return -EINVAL;
> }
>
> @@ -776,11 +785,17 @@ static int do_detach(int argc, char **argv)
> p_err("invalid attach type");
> return -EINVAL;
> }
> - NEXT_ARG();
> + if (attach_type != BPF_FLOW_DISSECTOR) {
> + NEXT_ARG();
> + if (!REQ_ARGS(2)) {
> + p_err("too few parameters for map detach");
> + return -EINVAL;
> + }
Would that make sense to factor argument checks or parsing for
do_attach() and do_detach() to some extent? In order to reduce the
number of attach-type-based exceptions to add in the code if we have
other attach types that do not take maps in the future.
>
> - mapfd = map_parse_fd(&argc, &argv);
> - if (mapfd < 0)
> - return mapfd;
> + mapfd = map_parse_fd(&argc, &argv);
> + if (mapfd < 0)
> + return mapfd;
> + }
>
> err = bpf_prog_detach2(progfd, mapfd, attach_type);
> if (err) {
> @@ -792,15 +807,16 @@ static int do_detach(int argc, char **argv)
> jsonw_null(json_wtr);
> return 0;
> }
> -static int do_load(int argc, char **argv)
> +
> +static int load_with_options(int argc, char **argv, bool first_prog_only)
> {
> enum bpf_attach_type expected_attach_type;
> struct bpf_object_open_attr attr = {
> .prog_type = BPF_PROG_TYPE_UNSPEC,
> };
> struct map_replace *map_replace = NULL;
> + struct bpf_program *prog = NULL, *pos;
> unsigned int old_map_fds = 0;
> - struct bpf_program *prog;
> struct bpf_object *obj;
> struct bpf_map *map;
> const char *pinfile;
> @@ -918,14 +934,20 @@ static int do_load(int argc, char **argv)
> goto err_free_reuse_maps;
> }
>
> - prog = bpf_program__next(NULL, obj);
> - if (!prog) {
> - p_err("object file doesn't contain any bpf program");
> - goto err_close_obj;
> + if (first_prog_only) {
> + prog = bpf_program__next(NULL, obj);
> + if (!prog) {
> + p_err("object file doesn't contain any bpf program");
> + goto err_close_obj;
> + }
> }
>
> - bpf_program__set_ifindex(prog, ifindex);
> if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
> + if (!prog) {
> + p_err("can not guess program type when loading all programs\n");
> + goto err_close_obj;
> + }
> +
> const char *sec_name = bpf_program__title(prog, false);
>
> err = libbpf_prog_type_by_name(sec_name, &attr.prog_type,
> @@ -936,8 +958,13 @@ static int do_load(int argc, char **argv)
> goto err_close_obj;
> }
> }
> - bpf_program__set_type(prog, attr.prog_type);
> - bpf_program__set_expected_attach_type(prog, expected_attach_type);
> +
> + bpf_object__for_each_program(pos, obj) {
> + bpf_program__set_ifindex(pos, ifindex);
> + bpf_program__set_type(pos, attr.prog_type);
> + bpf_program__set_expected_attach_type(pos,
> + expected_attach_type);
> + }
I still believe you can have programs of different types here, and be
able to load them. I tried it and managed to have it working fine. If no
type is provided from command line we can retrieve types for each
program from its section name. If a type is provided on the command
line, we can do the same, but I am not sure we should do it, or impose
that type for all programs instead.
>
> qsort(map_replace, old_map_fds, sizeof(*map_replace),
> map_replace_compar);
> @@ -1001,9 +1028,25 @@ static int do_load(int argc, char **argv)
> goto err_close_obj;
> }
>
> - if (do_pin_fd(bpf_program__fd(prog), pinfile))
> + err = mount_bpffs_for_pin(pinfile);
> + if (err)
> goto err_close_obj;
>
> + if (prog) {
Nit: Maybe "if (first_prog_only) {" instead? If I understand correctly,
at this stage it should be equivalent, but in my opinion it would make
it easier to understand why we have two cases here.
> + err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
> + if (err) {
> + p_err("failed to pin program %s",
> + bpf_program__title(prog, false));
> + goto err_close_obj;
> + }
> + } else {
> + err = bpf_object__pin(obj, pinfile);
> + if (err) {
> + p_err("failed to pin all programs");
> + goto err_close_obj;
> + }
> + }
> +
> if (json_output)
> jsonw_null(json_wtr);
>
> @@ -1023,6 +1066,16 @@ static int do_load(int argc, char **argv)
> return -1;
> }
>
> +static int do_load(int argc, char **argv)
> +{
> + return load_with_options(argc, argv, true);
> +}
> +
> +static int do_loadall(int argc, char **argv)
> +{
> + return load_with_options(argc, argv, false);
> +}
> +
> static int do_help(int argc, char **argv)
> {
> if (json_output) {
> @@ -1035,10 +1088,11 @@ static int do_help(int argc, char **argv)
> " %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
> " %s %s dump jited PROG [{ file FILE | opcodes }]\n"
> " %s %s pin PROG FILE\n"
> - " %s %s load OBJ FILE [type TYPE] [dev NAME] \\\n"
> + " %s %s { load | loadall } OBJ FILE \\\n"
> + " [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> - " %s %s attach PROG ATTACH_TYPE MAP\n"
> - " %s %s detach PROG ATTACH_TYPE MAP\n"
> + " %s %s attach PROG ATTACH_TYPE [MAP]\n"
> + " %s %s detach PROG ATTACH_TYPE [MAP]\n"
> " %s %s help\n"
> "\n"
> " " HELP_SPEC_MAP "\n"
> @@ -1050,7 +1104,8 @@ static int do_help(int argc, char **argv)
> " cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
> " cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
> " cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
> - " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse }\n"
> + " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
> + " flow_dissector }\n"
> " " HELP_SPEC_OPTIONS "\n"
> "",
> bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> @@ -1067,6 +1122,7 @@ static const struct cmd cmds[] = {
> { "dump", do_dump },
> { "pin", do_pin },
> { "load", do_load },
> + { "loadall", do_loadall },
> { "attach", do_attach },
> { "detach", do_detach },
> { 0 }
>
^ permalink raw reply
* [PATCH net-next v2 00/11] ICMP error handling for UDP tunnels
From: Stefano Brivio @ 2018-11-08 11:19 UTC (permalink / raw)
To: David S. Miller
Cc: Sabrina Dubroca, Xin Long, Stephen Hemminger, Jiri Benc,
David Ahern, netdev
This series introduces ICMP error handling for UDP tunnels and
encapsulations and related selftests. We need to handle ICMP errors to
support PMTU discovery and route redirection -- this support is entirely
missing right now:
- patch 1/11 adds a socket lookup for UDP tunnels that use, by design,
the same destination port on both endpoints -- i.e. VXLAN and GENEVE
- patches 2/11 to 7/11 are specific to VxLAN and GENEVE
- patches 8/11 and 9/11 add infrastructure for lookup of encapsulations
where sent packets cannot be matched via receiving socket lookup, i.e.
FoU and GUE
- patches 10/11 and 11/11 are specific to FoU and GUE
v2: changes are listed in the single patches
Stefano Brivio (11):
udp: Handle ICMP errors for tunnels with same destination port on both
endpoints
vxlan: ICMP error lookup handler
vxlan: Allow configuration of DF behaviour
selftests: pmtu: Introduce tests for IPv4/IPv6 over VXLAN over
IPv4/IPv6
geneve: ICMP error lookup handler
geneve: Allow configuration of DF behaviour
selftests: pmtu: Introduce tests for IPv4/IPv6 over GENEVE over
IPv4/IPv6
net: Convert protocol error handlers from void to int
udp: Support for error handlers of tunnels with arbitrary destination
port
fou, fou6: ICMP error handlers for FoU and GUE
selftests: pmtu: Introduce FoU and GUE PMTU exceptions tests
drivers/net/geneve.c | 107 +++++++-
drivers/net/vxlan.c | 58 ++++-
include/linux/udp.h | 1 +
include/net/icmp.h | 2 +-
include/net/ip6_tunnel.h | 2 +
include/net/ip_tunnels.h | 1 +
include/net/protocol.h | 9 +-
include/net/sctp/sctp.h | 2 +-
include/net/tcp.h | 2 +-
include/net/udp.h | 2 +-
include/net/udp_tunnel.h | 3 +
include/net/vxlan.h | 1 +
include/uapi/linux/if_link.h | 18 ++
net/dccp/ipv4.c | 13 +-
net/dccp/ipv6.c | 13 +-
net/ipv4/fou.c | 68 +++++
net/ipv4/gre_demux.c | 9 +-
net/ipv4/icmp.c | 6 +-
net/ipv4/ip_gre.c | 48 ++--
net/ipv4/ipip.c | 14 +-
net/ipv4/protocol.c | 1 +
net/ipv4/tcp_ipv4.c | 22 +-
net/ipv4/tunnel4.c | 18 +-
net/ipv4/udp.c | 121 ++++++++-
net/ipv4/udp_impl.h | 2 +-
net/ipv4/udp_tunnel.c | 1 +
net/ipv4/udplite.c | 4 +-
net/ipv4/xfrm4_protocol.c | 18 +-
net/ipv6/fou6.c | 74 ++++++
net/ipv6/icmp.c | 4 +-
net/ipv6/ip6_gre.c | 18 +-
net/ipv6/tcp_ipv6.c | 13 +-
net/ipv6/tunnel6.c | 12 +-
net/ipv6/udp.c | 146 +++++++++--
net/ipv6/udp_impl.h | 4 +-
net/ipv6/udplite.c | 5 +-
net/ipv6/xfrm6_protocol.c | 18 +-
net/sctp/input.c | 5 +-
net/sctp/ipv6.c | 7 +-
tools/testing/selftests/net/pmtu.sh | 377 ++++++++++++++++++++++++++--
40 files changed, 1083 insertions(+), 166 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH net-next v2 01/11] udp: Handle ICMP errors for tunnels with same destination port on both endpoints
From: Stefano Brivio @ 2018-11-08 11:19 UTC (permalink / raw)
To: David S. Miller
Cc: Sabrina Dubroca, Xin Long, Stephen Hemminger, Jiri Benc,
David Ahern, netdev
In-Reply-To: <cover.1541675666.git.sbrivio@redhat.com>
For both IPv4 and IPv6, if we can't match errors to a socket, try
tunnels before ignoring them. Look up a socket with the original source
and destination ports as found in the UDP packet inside the ICMP payload,
this will work for tunnels that force the same destination port for both
endpoints, i.e. VXLAN and GENEVE.
Actually, lwtunnels could break this assumption if they are configured by
an external control plane to have different destination ports on the
endpoints: in this case, we won't be able to trace ICMP messages back to
them.
For IPv6 redirect messages, call ip6_redirect() directly with the output
interface argument set to the interface we received the packet from (as
it's the very interface we should build the exception on), otherwise the
new nexthop will be rejected. There's no such need for IPv4.
Tunnels can now export an encap_err_lookup() operation that indicates a
match. Pass the packet to the lookup function, and if the tunnel driver
reports a matching association, continue with regular ICMP error handling.
v2:
- Added newline between network and transport header sets in
__udp{4,6}_lib_err_encap() (David Miller)
- Removed redundant skb_reset_network_header(skb); in
__udp4_lib_err_encap()
- Removed redundant reassignment of iph in __udp4_lib_err_encap()
(Sabrina Dubroca)
- Edited comment to __udp{4,6}_lib_err_encap() to reflect the fact this
won't work with lwtunnels configured to use asymmetric ports. By the way,
it's VXLAN, not VxLAN (Jiri Benc)
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
include/linux/udp.h | 1 +
include/net/udp_tunnel.h | 3 ++
net/ipv4/udp.c | 79 +++++++++++++++++++++++++++++++----
net/ipv4/udp_tunnel.c | 1 +
net/ipv6/udp.c | 89 +++++++++++++++++++++++++++++++++++-----
5 files changed, 153 insertions(+), 20 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 0a9c54e76305..2725c83395bf 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -77,6 +77,7 @@ struct udp_sock {
* For encapsulation sockets.
*/
int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+ int (*encap_err_lookup)(struct sock *sk, struct sk_buff *skb);
void (*encap_destroy)(struct sock *sk);
/* GRO functions for UDP socket */
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 3fbe56430e3b..dc8d804af3b4 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -64,6 +64,8 @@ static inline int udp_sock_create(struct net *net,
}
typedef int (*udp_tunnel_encap_rcv_t)(struct sock *sk, struct sk_buff *skb);
+typedef int (*udp_tunnel_encap_err_lookup_t)(struct sock *sk,
+ struct sk_buff *skb);
typedef void (*udp_tunnel_encap_destroy_t)(struct sock *sk);
typedef struct sk_buff *(*udp_tunnel_gro_receive_t)(struct sock *sk,
struct list_head *head,
@@ -76,6 +78,7 @@ struct udp_tunnel_sock_cfg {
/* Used for setting up udp_sock fields, see udp.h for details */
__u8 encap_type;
udp_tunnel_encap_rcv_t encap_rcv;
+ udp_tunnel_encap_err_lookup_t encap_err_lookup;
udp_tunnel_encap_destroy_t encap_destroy;
udp_tunnel_gro_receive_t gro_receive;
udp_tunnel_gro_complete_t gro_complete;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 3488650b90ac..ce759b61f6cd 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -583,6 +583,62 @@ static inline bool __udp_is_mcast_sock(struct net *net, struct sock *sk,
return true;
}
+DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
+void udp_encap_enable(void)
+{
+ static_branch_enable(&udp_encap_needed_key);
+}
+EXPORT_SYMBOL(udp_encap_enable);
+
+/* Try to match ICMP errors to UDP tunnels by looking up a socket without
+ * reversing source and destination port: this will match tunnels that force the
+ * same destination port on both endpoints (e.g. VXLAN, GENEVE). Note that
+ * lwtunnels might actually break this assumption by being configured with
+ * different destination ports on endpoints, in this case we won't be able to
+ * trace ICMP messages back to them.
+ *
+ * Then ask the tunnel implementation to match the error against a valid
+ * association.
+ *
+ * Return the socket if we have a match.
+ */
+static struct sock *__udp4_lib_err_encap(struct net *net,
+ const struct iphdr *iph,
+ struct udphdr *uh,
+ struct udp_table *udptable,
+ struct sk_buff *skb)
+{
+ int (*lookup)(struct sock *sk, struct sk_buff *skb);
+ int network_offset, transport_offset;
+ struct udp_sock *up;
+ struct sock *sk;
+
+ sk = __udp4_lib_lookup(net, iph->daddr, uh->source,
+ iph->saddr, uh->dest, skb->dev->ifindex, 0,
+ udptable, NULL);
+ if (!sk)
+ return NULL;
+
+ network_offset = skb_network_offset(skb);
+ transport_offset = skb_transport_offset(skb);
+
+ /* Network header needs to point to the outer IPv4 header inside ICMP */
+ skb_reset_network_header(skb);
+
+ /* Transport header needs to point to the UDP header */
+ skb_set_transport_header(skb, iph->ihl << 2);
+
+ up = udp_sk(sk);
+ lookup = READ_ONCE(up->encap_err_lookup);
+ if (!lookup || lookup(sk, skb))
+ sk = NULL;
+
+ skb_set_transport_header(skb, transport_offset);
+ skb_set_network_header(skb, network_offset);
+
+ return sk;
+}
+
/*
* This routine is called by the ICMP module when it gets some
* sort of error condition. If err < 0 then the socket should
@@ -601,6 +657,7 @@ void __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
struct udphdr *uh = (struct udphdr *)(skb->data+(iph->ihl<<2));
const int type = icmp_hdr(skb)->type;
const int code = icmp_hdr(skb)->code;
+ bool tunnel = false;
struct sock *sk;
int harderr;
int err;
@@ -610,8 +667,15 @@ void __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
iph->saddr, uh->source, skb->dev->ifindex,
inet_sdif(skb), udptable, NULL);
if (!sk) {
- __ICMP_INC_STATS(net, ICMP_MIB_INERRORS);
- return; /* No socket for error */
+ /* No socket for error: try tunnels before discarding */
+ if (static_branch_unlikely(&udp_encap_needed_key))
+ sk = __udp4_lib_err_encap(net, iph, uh, udptable, skb);
+
+ if (!sk) {
+ __ICMP_INC_STATS(net, ICMP_MIB_INERRORS);
+ return;
+ }
+ tunnel = true;
}
err = 0;
@@ -654,6 +718,10 @@ void __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
* RFC1122: OK. Passes ICMP errors back to application, as per
* 4.1.3.3.
*/
+ if (tunnel) {
+ /* ...not for tunnels though: we don't have a sending socket */
+ goto out;
+ }
if (!inet->recverr) {
if (!harderr || sk->sk_state != TCP_ESTABLISHED)
goto out;
@@ -1891,13 +1959,6 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
return 0;
}
-DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
-void udp_encap_enable(void)
-{
- static_branch_enable(&udp_encap_needed_key);
-}
-EXPORT_SYMBOL(udp_encap_enable);
-
/* returns:
* -1: error
* 0: success
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index 6539ff15e9a3..d0c412fc56ad 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -68,6 +68,7 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
udp_sk(sk)->encap_type = cfg->encap_type;
udp_sk(sk)->encap_rcv = cfg->encap_rcv;
+ udp_sk(sk)->encap_err_lookup = cfg->encap_err_lookup;
udp_sk(sk)->encap_destroy = cfg->encap_destroy;
udp_sk(sk)->gro_receive = cfg->gro_receive;
udp_sk(sk)->gro_complete = cfg->gro_complete;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index c55698d19d68..1216c920f945 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -462,6 +462,61 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
goto try_again;
}
+DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
+void udpv6_encap_enable(void)
+{
+ static_branch_enable(&udpv6_encap_needed_key);
+}
+EXPORT_SYMBOL(udpv6_encap_enable);
+
+/* Try to match ICMP errors to UDP tunnels by looking up a socket without
+ * reversing source and destination port: this will match tunnels that force the
+ * same destination port on both endpoints (e.g. VXLAN, GENEVE). Note that
+ * lwtunnels might actually break this assumption by being configured with
+ * different destination ports on endpoints, in this case we won't be able to
+ * trace ICMP messages back to them.
+ *
+ * Then ask the tunnel implementation to match the error against a valid
+ * association.
+ *
+ * Return the socket if we have a match.
+ */
+static struct sock *__udp6_lib_err_encap(struct net *net,
+ const struct ipv6hdr *hdr, int offset,
+ struct udphdr *uh,
+ struct udp_table *udptable,
+ struct sk_buff *skb)
+{
+ int (*lookup)(struct sock *sk, struct sk_buff *skb);
+ int network_offset, transport_offset;
+ struct udp_sock *up;
+ struct sock *sk;
+
+ sk = __udp6_lib_lookup(net, &hdr->daddr, uh->source,
+ &hdr->saddr, uh->dest,
+ inet6_iif(skb), 0, udptable, skb);
+ if (!sk)
+ return NULL;
+
+ network_offset = skb_network_offset(skb);
+ transport_offset = skb_transport_offset(skb);
+
+ /* Network header needs to point to the outer IPv6 header inside ICMP */
+ skb_reset_network_header(skb);
+
+ /* Transport header needs to point to the UDP header */
+ skb_set_transport_header(skb, offset);
+
+ up = udp_sk(sk);
+ lookup = READ_ONCE(up->encap_err_lookup);
+ if (!lookup || lookup(sk, skb))
+ sk = NULL;
+
+ skb_set_transport_header(skb, transport_offset);
+ skb_set_network_header(skb, network_offset);
+ return sk;
+}
+
void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
u8 type, u8 code, int offset, __be32 info,
struct udp_table *udptable)
@@ -471,6 +526,7 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
const struct in6_addr *saddr = &hdr->saddr;
const struct in6_addr *daddr = &hdr->daddr;
struct udphdr *uh = (struct udphdr *)(skb->data+offset);
+ bool tunnel = false;
struct sock *sk;
int harderr;
int err;
@@ -479,9 +535,18 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
sk = __udp6_lib_lookup(net, daddr, uh->dest, saddr, uh->source,
inet6_iif(skb), inet6_sdif(skb), udptable, skb);
if (!sk) {
- __ICMP6_INC_STATS(net, __in6_dev_get(skb->dev),
- ICMP6_MIB_INERRORS);
- return;
+ /* No socket for error: try tunnels before discarding */
+ if (static_branch_unlikely(&udpv6_encap_needed_key)) {
+ sk = __udp6_lib_err_encap(net, hdr, offset, uh,
+ udptable, skb);
+ }
+
+ if (!sk) {
+ __ICMP6_INC_STATS(net, __in6_dev_get(skb->dev),
+ ICMP6_MIB_INERRORS);
+ return;
+ }
+ tunnel = true;
}
harderr = icmpv6_err_convert(type, code, &err);
@@ -495,10 +560,19 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
harderr = 1;
}
if (type == NDISC_REDIRECT) {
- ip6_sk_redirect(skb, sk);
+ if (tunnel) {
+ ip6_redirect(skb, sock_net(sk), inet6_iif(skb),
+ sk->sk_mark, sk->sk_uid);
+ } else {
+ ip6_sk_redirect(skb, sk);
+ }
goto out;
}
+ /* Tunnels don't have an application socket: don't pass errors back */
+ if (tunnel)
+ goto out;
+
if (!np->recverr) {
if (!harderr || sk->sk_state != TCP_ESTABLISHED)
goto out;
@@ -547,13 +621,6 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
}
-DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
-void udpv6_encap_enable(void)
-{
- static_branch_enable(&udpv6_encap_needed_key);
-}
-EXPORT_SYMBOL(udpv6_encap_enable);
-
static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
--
2.19.1
^ permalink raw reply related
* [PATCH net-next v2 02/11] vxlan: ICMP error lookup handler
From: Stefano Brivio @ 2018-11-08 11:19 UTC (permalink / raw)
To: David S. Miller
Cc: Sabrina Dubroca, Xin Long, Stephen Hemminger, Jiri Benc,
David Ahern, netdev
In-Reply-To: <cover.1541675666.git.sbrivio@redhat.com>
Export an encap_err_lookup() operation to match an ICMP error against a
valid VNI.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: no changes
drivers/net/vxlan.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ae969f806d56..0851af6733f3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1552,6 +1552,34 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
+/* Callback from net/ipv{4,6}/udp.c to check that we have a VNI for errors */
+static int vxlan_err_lookup(struct sock *sk, struct sk_buff *skb)
+{
+ struct vxlan_dev *vxlan;
+ struct vxlan_sock *vs;
+ struct vxlanhdr *hdr;
+ __be32 vni;
+
+ if (skb->len < VXLAN_HLEN)
+ return -EINVAL;
+
+ hdr = vxlan_hdr(skb);
+
+ if (!(hdr->vx_flags & VXLAN_HF_VNI))
+ return -EINVAL;
+
+ vs = rcu_dereference_sk_user_data(sk);
+ if (!vs)
+ return -ENOENT;
+
+ vni = vxlan_vni(hdr->vx_vni);
+ vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni);
+ if (!vxlan)
+ return -ENOENT;
+
+ return 0;
+}
+
static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
{
struct vxlan_dev *vxlan = netdev_priv(dev);
@@ -2948,6 +2976,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
tunnel_cfg.sk_user_data = vs;
tunnel_cfg.encap_type = 1;
tunnel_cfg.encap_rcv = vxlan_rcv;
+ tunnel_cfg.encap_err_lookup = vxlan_err_lookup;
tunnel_cfg.encap_destroy = NULL;
tunnel_cfg.gro_receive = vxlan_gro_receive;
tunnel_cfg.gro_complete = vxlan_gro_complete;
--
2.19.1
^ permalink raw reply related
* [PATCH net-next v2 03/11] vxlan: Allow configuration of DF behaviour
From: Stefano Brivio @ 2018-11-08 11:19 UTC (permalink / raw)
To: David S. Miller
Cc: Sabrina Dubroca, Xin Long, Stephen Hemminger, Jiri Benc,
David Ahern, netdev
In-Reply-To: <cover.1541675666.git.sbrivio@redhat.com>
Allow users to set the IPv4 DF bit in outgoing packets, or to inherit its
value from the IPv4 inner header. If the encapsulated protocol is IPv6 and
DF is configured to be inherited, always set it.
For IPv4, inheriting DF from the inner header was probably intended from
the very beginning judging by the comment to vxlan_xmit(), but it wasn't
actually implemented -- also because it would have done more harm than
good, without handling for ICMP Fragmentation Needed messages.
According to RFC 7348, "Path MTU discovery MAY be used". An expired RFC
draft, draft-saum-nvo3-pmtud-over-vxlan-05, whose purpose was to describe
PMTUD implementation, says that "is a MUST that Vxlan gateways [...]
SHOULD set the DF-bit [...]", whatever that means.
Given this background, the only sane option is probably to let the user
decide, and keep the current behaviour as default.
This only applies to non-lwt tunnels: if an external control plane is
used, tunnel key will still control the DF flag.
v2:
- DF behaviour configuration only applies for non-lwt tunnels, move DF
setting to if (!info) block in vxlan_xmit_one() (Stephen Hemminger)
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/vxlan.c | 29 ++++++++++++++++++++++++++++-
include/net/vxlan.h | 1 +
include/uapi/linux/if_link.h | 9 +++++++++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 0851af6733f3..c3e65e78f015 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2278,13 +2278,24 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
goto tx_error;
}
- /* Bypass encapsulation if the destination is local */
if (!info) {
+ /* Bypass encapsulation if the destination is local */
err = encap_bypass_if_local(skb, dev, vxlan, dst,
dst_port, ifindex, vni,
&rt->dst, rt->rt_flags);
if (err)
goto out_unlock;
+
+ if (vxlan->cfg.df == VXLAN_DF_SET) {
+ df = htons(IP_DF);
+ } else if (vxlan->cfg.df == VXLAN_DF_INHERIT) {
+ struct ethhdr *eth = eth_hdr(skb);
+
+ if (ntohs(eth->h_proto) == ETH_P_IPV6 ||
+ (ntohs(eth->h_proto) == ETH_P_IP &&
+ old_iph->frag_off & htons(IP_DF)))
+ df = htons(IP_DF);
+ }
} else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT) {
df = htons(IP_DF);
}
@@ -2837,6 +2848,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
[IFLA_VXLAN_GPE] = { .type = NLA_FLAG, },
[IFLA_VXLAN_REMCSUM_NOPARTIAL] = { .type = NLA_FLAG },
[IFLA_VXLAN_TTL_INHERIT] = { .type = NLA_FLAG },
+ [IFLA_VXLAN_DF] = { .type = NLA_U8 },
};
static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
@@ -2893,6 +2905,16 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
}
}
+ if (data[IFLA_VXLAN_DF]) {
+ enum ifla_vxlan_df df = nla_get_u8(data[IFLA_VXLAN_DF]);
+
+ if (df < 0 || df > VXLAN_DF_MAX) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_DF],
+ "Invalid DF attribute");
+ return -EINVAL;
+ }
+ }
+
return 0;
}
@@ -3538,6 +3560,9 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
conf->mtu = nla_get_u32(tb[IFLA_MTU]);
}
+ if (data[IFLA_VXLAN_DF])
+ conf->df = nla_get_u8(data[IFLA_VXLAN_DF]);
+
return 0;
}
@@ -3630,6 +3655,7 @@ static size_t vxlan_get_size(const struct net_device *dev)
nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TTL */
nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TTL_INHERIT */
nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_TOS */
+ nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_DF */
nla_total_size(sizeof(__be32)) + /* IFLA_VXLAN_LABEL */
nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_LEARNING */
nla_total_size(sizeof(__u8)) + /* IFLA_VXLAN_PROXY */
@@ -3696,6 +3722,7 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_u8(skb, IFLA_VXLAN_TTL_INHERIT,
!!(vxlan->cfg.flags & VXLAN_F_TTL_INHERIT)) ||
nla_put_u8(skb, IFLA_VXLAN_TOS, vxlan->cfg.tos) ||
+ nla_put_u8(skb, IFLA_VXLAN_DF, vxlan->cfg.df) ||
nla_put_be32(skb, IFLA_VXLAN_LABEL, vxlan->cfg.label) ||
nla_put_u8(skb, IFLA_VXLAN_LEARNING,
!!(vxlan->cfg.flags & VXLAN_F_LEARN)) ||
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 03431c148e16..ec999c49df1f 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -216,6 +216,7 @@ struct vxlan_config {
unsigned long age_interval;
unsigned int addrmax;
bool no_share;
+ enum ifla_vxlan_df df;
};
struct vxlan_dev_node {
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 1debfa42cba1..efc588949431 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -533,6 +533,7 @@ enum {
IFLA_VXLAN_LABEL,
IFLA_VXLAN_GPE,
IFLA_VXLAN_TTL_INHERIT,
+ IFLA_VXLAN_DF,
__IFLA_VXLAN_MAX
};
#define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
@@ -542,6 +543,14 @@ struct ifla_vxlan_port_range {
__be16 high;
};
+enum ifla_vxlan_df {
+ VXLAN_DF_UNSET = 0,
+ VXLAN_DF_SET,
+ VXLAN_DF_INHERIT,
+ __VXLAN_DF_END,
+ VXLAN_DF_MAX = __VXLAN_DF_END - 1,
+};
+
/* GENEVE section */
enum {
IFLA_GENEVE_UNSPEC,
--
2.19.1
^ permalink raw reply related
* [PATCH net-next v2 04/11] selftests: pmtu: Introduce tests for IPv4/IPv6 over VXLAN over IPv4/IPv6
From: Stefano Brivio @ 2018-11-08 11:19 UTC (permalink / raw)
To: David S. Miller
Cc: Sabrina Dubroca, Xin Long, Stephen Hemminger, Jiri Benc,
David Ahern, netdev
In-Reply-To: <cover.1541675666.git.sbrivio@redhat.com>
Use a router between endpoints, implemented via namespaces, set a low MTU
between router and destination endpoint, exceed it and check PMTU value in
route exceptions.
v2:
- Change all occurrences of VxLAN to VXLAN (Jiri Benc)
- Introduce IPv4 tests right away, if iproute2 doesn't support the 'df'
link option they will be skipped (David Ahern)
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
tools/testing/selftests/net/pmtu.sh | 143 ++++++++++++++++++++++++----
1 file changed, 125 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index a369d616b390..33cba295ad45 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -26,6 +26,23 @@
# - pmtu_ipv6
# Same as pmtu_ipv4, except for locked PMTU tests, using IPv6
#
+# - pmtu_ipv4_vxlan4_exception
+# Set up the same network topology as pmtu_ipv4, create a VXLAN tunnel
+# over IPv4 between A and B, routed via R1. On the link between R1 and B,
+# set a MTU lower than the VXLAN MTU and the MTU on the link between A and
+# R1. Send IPv4 packets, exceeding the MTU between R1 and B, over VXLAN
+# from A to B and check that the PMTU exception is created with the right
+# value on A
+#
+# - pmtu_ipv6_vxlan4_exception
+# Same as pmtu_ipv4_vxlan4_exception, but send IPv6 packets from A to B
+#
+# - pmtu_ipv4_vxlan6_exception
+# Same as pmtu_ipv4_vxlan4_exception, but use IPv6 transport from A to B
+#
+# - pmtu_ipv6_vxlan6_exception
+# Same as pmtu_ipv4_vxlan6_exception, but send IPv6 packets from A to B
+#
# - pmtu_vti4_exception
# Set up vti tunnel on top of veth, with xfrm states and policies, in two
# namespaces with matching endpoints. Check that route exception is not
@@ -72,6 +89,10 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
tests="
pmtu_ipv4_exception ipv4: PMTU exceptions
pmtu_ipv6_exception ipv6: PMTU exceptions
+ pmtu_ipv4_vxlan4_exception IPv4 over vxlan4: PMTU exceptions
+ pmtu_ipv6_vxlan4_exception IPv6 over vxlan4: PMTU exceptions
+ pmtu_ipv4_vxlan6_exception IPv4 over vxlan6: PMTU exceptions
+ pmtu_ipv6_vxlan6_exception IPv6 over vxlan6: PMTU exceptions
pmtu_vti6_exception vti6: PMTU exceptions
pmtu_vti4_exception vti4: PMTU exceptions
pmtu_vti4_default_mtu vti4: default MTU assignment
@@ -95,8 +116,8 @@ ns_r2="ip netns exec ${NS_R2}"
# Addresses are:
# - IPv4: PREFIX4.SEGMENT.ID (/24)
# - IPv6: PREFIX6:SEGMENT::ID (/64)
-prefix4="192.168"
-prefix6="fd00"
+prefix4="10.0"
+prefix6="fc00"
a_r1=1
a_r2=2
b_r1=3
@@ -129,12 +150,12 @@ veth6_a_addr="fd00:1::a"
veth6_b_addr="fd00:1::b"
veth6_mask="64"
-vti4_a_addr="192.168.2.1"
-vti4_b_addr="192.168.2.2"
-vti4_mask="24"
-vti6_a_addr="fd00:2::a"
-vti6_b_addr="fd00:2::b"
-vti6_mask="64"
+tunnel4_a_addr="192.168.2.1"
+tunnel4_b_addr="192.168.2.2"
+tunnel4_mask="24"
+tunnel6_a_addr="fd00:2::a"
+tunnel6_b_addr="fd00:2::b"
+tunnel6_mask="64"
dummy6_0_addr="fc00:1000::0"
dummy6_1_addr="fc00:1001::0"
@@ -202,11 +223,39 @@ setup_vti() {
}
setup_vti4() {
- setup_vti 4 ${veth4_a_addr} ${veth4_b_addr} ${vti4_a_addr} ${vti4_b_addr} ${vti4_mask}
+ setup_vti 4 ${veth4_a_addr} ${veth4_b_addr} ${tunnel4_a_addr} ${tunnel4_b_addr} ${tunnel4_mask}
}
setup_vti6() {
- setup_vti 6 ${veth6_a_addr} ${veth6_b_addr} ${vti6_a_addr} ${vti6_b_addr} ${vti6_mask}
+ setup_vti 6 ${veth6_a_addr} ${veth6_b_addr} ${tunnel6_a_addr} ${tunnel6_b_addr} ${tunnel6_mask}
+}
+
+setup_vxlan() {
+ a_addr="${1}"
+ b_addr="${2}"
+ opts="${3}"
+
+ ${ns_a} ip link add vxlan_a type vxlan id 1 local ${a_addr} remote ${b_addr} ttl 64 dstport 4789 ${opts} || return 1
+ ${ns_b} ip link add vxlan_b type vxlan id 1 local ${b_addr} remote ${a_addr} ttl 64 dstport 4789 ${opts}
+
+ ${ns_a} ip addr add ${tunnel4_a_addr}/${tunnel4_mask} dev vxlan_a
+ ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev vxlan_b
+
+ ${ns_a} ip addr add ${tunnel6_a_addr}/${tunnel6_mask} dev vxlan_a
+ ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev vxlan_b
+
+ ${ns_a} ip link set vxlan_a up
+ ${ns_b} ip link set vxlan_b up
+
+ sleep 1
+}
+
+setup_vxlan4() {
+ setup_vxlan ${prefix4}.${a_r1}.1 ${prefix4}.${b_r1}.1 "df set"
+}
+
+setup_vxlan6() {
+ setup_vxlan ${prefix6}:${a_r1}::1 ${prefix6}:${b_r1}::1 ""
}
setup_xfrm() {
@@ -465,6 +514,64 @@ test_pmtu_ipv6_exception() {
test_pmtu_ipvX 6
}
+test_pmtu_ipvX_over_vxlanY_exception() {
+ family=${1}
+ outer_family=${2}
+ ll_mtu=4000
+
+ if [ ${outer_family} -eq 4 ]; then
+ setup namespaces routing vxlan4 || return 2
+ # IPv4 header UDP header VXLAN header Ethernet header
+ exp_mtu=$((${ll_mtu} - 20 - 8 - 8 - 14))
+ else
+ setup namespaces routing vxlan6 || return 2
+ # IPv6 header UDP header VXLAN header Ethernet header
+ exp_mtu=$((${ll_mtu} - 40 - 8 - 8 - 14))
+ fi
+
+ trace "${ns_a}" vxlan_a "${ns_b}" vxlan_b \
+ "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \
+ "${ns_b}" veth_B-R1 "${ns_r1}" veth_R1-B
+
+ if [ ${family} -eq 4 ]; then
+ ping=ping
+ dst=${tunnel4_b_addr}
+ else
+ ping=${ping6}
+ dst=${tunnel6_b_addr}
+ fi
+
+ # Create route exception by exceeding link layer MTU
+ mtu "${ns_a}" veth_A-R1 $((${ll_mtu} + 1000))
+ mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
+ mtu "${ns_b}" veth_B-R1 ${ll_mtu}
+ mtu "${ns_r1}" veth_R1-B ${ll_mtu}
+
+ mtu "${ns_a}" vxlan_a $((${ll_mtu} + 1000))
+ mtu "${ns_b}" vxlan_b $((${ll_mtu} + 1000))
+ ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
+
+ # Check that exception was created
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
+ check_pmtu_value ${exp_mtu} "${pmtu}" "exceeding link layer MTU on VXLAN interface"
+}
+
+test_pmtu_ipv4_vxlan4_exception() {
+ test_pmtu_ipvX_over_vxlanY_exception 4 4
+}
+
+test_pmtu_ipv6_vxlan4_exception() {
+ test_pmtu_ipvX_over_vxlanY_exception 6 4
+}
+
+test_pmtu_ipv4_vxlan6_exception() {
+ test_pmtu_ipvX_over_vxlanY_exception 4 6
+}
+
+test_pmtu_ipv6_vxlan6_exception() {
+ test_pmtu_ipvX_over_vxlanY_exception 6 6
+}
+
test_pmtu_vti4_exception() {
setup namespaces veth vti4 xfrm4 || return 2
trace "${ns_a}" veth_a "${ns_b}" veth_b \
@@ -484,14 +591,14 @@ test_pmtu_vti4_exception() {
# Send DF packet without exceeding link layer MTU, check that no
# exception is created
- ${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${vti4_b_addr} > /dev/null
- pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
+ ${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1
# Now exceed link layer MTU by one byte, check that exception is created
# with the right PMTU value
- ${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${vti4_b_addr} > /dev/null
- pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
+ ${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))"
}
@@ -506,20 +613,20 @@ test_pmtu_vti6_exception() {
mtu "${ns_b}" veth_b 4000
mtu "${ns_a}" vti6_a 5000
mtu "${ns_b}" vti6_b 5000
- ${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${vti6_b_addr} > /dev/null
+ ${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${tunnel6_b_addr} > /dev/null
# Check that exception was created
- pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
check_pmtu_value any "${pmtu}" "creating tunnel exceeding link layer MTU" || return 1
# Decrease tunnel MTU, check for PMTU decrease in route exception
mtu "${ns_a}" vti6_a 3000
- pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
check_pmtu_value "3000" "${pmtu}" "decreasing tunnel MTU" || fail=1
# Increase tunnel MTU, check for PMTU increase in route exception
mtu "${ns_a}" vti6_a 9000
- pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
check_pmtu_value "9000" "${pmtu}" "increasing tunnel MTU" || fail=1
return ${fail}
--
2.19.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox