* RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Rodney Cummings @ 2017-10-16 16:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <20171016164737.GD13339@lunn.ch>
PAUSE is only one higher layer entity. The source MAC address may not matter there, but it definitely matters for other higher layer entities (like IEEE Std 1588).
The Marvell switch can run more than just PAUSE as a higher layer protocol.
How do you guarantee that this MAC address is used for PAUSE, and only for PAUSE?
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, October 16, 2017 11:48 AM
> To: Rodney Cummings <rodney.cummings@ni.com>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@savoirfairelinux.com; David S. Miller <davem@davemloft.net>;
> Florian Fainelli <f.fainelli@gmail.com>; David Laight
> <David.Laight@ACULAB.COM>
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
>
> On Mon, Oct 16, 2017 at 04:28:04PM +0000, Rodney Cummings wrote:
> > Hi Andrew,
> >
> > I may have misunderstood.
> >
> > If this MAC address is the destination
>
> Nope. This is the source address, for Pause frames.
>
> > My concern is that for a source MAC address, a local random MAC
> > address is not safe in all networks, because it has the potential
> > for duplication. That topic has been discussed quite a bit in IEEE
> > 802.
>
> Duplications don't matter, for pause frames. The source address
> appears to be unused. And these frames don't get passed the direct
> peers MAC layer.
>
> Andrew
^ permalink raw reply
* Re: [PATCH net] bpf: disallow arithmetic operations on context pointer
From: Edward Cree @ 2017-10-16 16:47 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <20171016093043.36fcfedc@cakuba.netronome.com>
On 16/10/17 17:30, Jakub Kicinski wrote:
> On Mon, 16 Oct 2017 17:16:24 +0100, Edward Cree wrote:
>> On 16/10/17 16:45, Jakub Kicinski wrote:
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 8b8d6ba39e23..8499759d0c7a 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -1116,7 +1116,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>>> /* ctx accesses must be at a fixed offset, so that we can
>>> * determine what type of data were returned.
>>> */
>>> - if (!tnum_is_const(reg->var_off)) {
>>> + if (reg->off) {
>>> + verbose("derefence of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
>> This is slightly unclear, it's not that two adds is bad (e.g. r1 += 8;
>> r0 = *(u32 *)r1 is bad too), it's that the offset must be in the load,
>> not the register; your message might be accurate for some compilers but
>> not in full generality (especially for assemblers without compiling).
> I'm happy to hear better suggestions :) I've spent quite a bit of time
> scratching my head thinking how to phrase this best. The first
> part of the message is general enough IMHO, the second is targeted
> mostly at C developers.
Hmm, what really bugs me is that if e.g. the compiler turned
*(ctx + 4 + 4)
or
ctx[4 + 4]
or even
ctx->arraymemb[4]
into this kind of arithmetic on ctx, arguably that would be a bug in the
compiler — if it's doing proper constexpr folding on its IR (or something
along those lines) it should be able to turn them all into good LDX. The
same even goes for if (ctx + 4) got stored in a local, because there's no
reason that has to map to a register.
So it's not even that "your C source breaks the rules", it's that "your C
compiler did something silly that we don't handle".
Maybe the message should be "compiler maybe mishandled ctx+const+const"?
-Ed
^ permalink raw reply
* Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Andrew Lunn @ 2017-10-16 16:47 UTC (permalink / raw)
To: Rodney Cummings
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <CY1PR0401MB1536097366C596C42A8B225B924F0@CY1PR0401MB1536.namprd04.prod.outlook.com>
On Mon, Oct 16, 2017 at 04:28:04PM +0000, Rodney Cummings wrote:
> Hi Andrew,
>
> I may have misunderstood.
>
> If this MAC address is the destination
Nope. This is the source address, for Pause frames.
> My concern is that for a source MAC address, a local random MAC
> address is not safe in all networks, because it has the potential
> for duplication. That topic has been discussed quite a bit in IEEE
> 802.
Duplications don't matter, for pause frames. The source address
appears to be unused. And these frames don't get passed the direct
peers MAC layer.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Andrew Lunn @ 2017-10-16 16:44 UTC (permalink / raw)
To: David Laight
Cc: Rodney Cummings, Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD00980E8@AcuExch.aculab.com>
On Mon, Oct 16, 2017 at 04:30:34PM +0000, David Laight wrote:
> From: Andrew Lunn
> > Sent: 16 October 2017 17:10
> ...
> > So, received Pause frames never leave the MAC. They don't get bridged,
> > nor do they get passed up for host processing. They are purely point
> > to point between two MAC peers. The destination is unambiguous. It is
> > simple the other MAC peer. The destination address makes it clear it
> > is a pause frame, the the source address seems to be unneeded.
> >
> > In this context, a random MAC addresses are safe.
>
> Is there any reason why a fixed value (say 00:00:00:00:00:00)
> can't be used?
I was going to suggest 42:42:42:42:42:42 :-)
Andrew
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: implement VIRTIO_CONFIG_S_NEEDS_RESET
From: Michael S. Tsirkin @ 2017-10-16 16:38 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, David Miller, Jason Wang, virtualization,
Willem de Bruijn
In-Reply-To: <CAF=yD-LW7f30V8Ci1SQUPZ-WBihqr93cWQP2QBFg2oQtytvAPg@mail.gmail.com>
On Mon, Oct 16, 2017 at 12:04:57PM -0400, Willem de Bruijn wrote:
> On Mon, Oct 16, 2017 at 11:31 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Oct 16, 2017 at 11:03:18AM -0400, Willem de Bruijn wrote:
> >> >> +static int virtnet_reset(struct virtnet_info *vi)
> >> >> +{
> >> >> + struct virtio_device *dev = vi->vdev;
> >> >> + int ret;
> >> >> +
> >> >> + virtio_config_disable(dev);
> >> >> + dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
> >> >> + virtnet_freeze_down(dev, true);
> >> >> + remove_vq_common(vi);
> >> >> +
> >> >> + virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> >> >> + virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
> >> >> +
> >> >> + ret = virtio_finalize_features(dev);
> >> >> + if (ret)
> >> >> + goto err;
> >> >> +
> >> >> + ret = virtnet_restore_up(dev);
> >> >> + if (ret)
> >> >> + goto err;
> >> >> +
> >> >> + ret = virtnet_set_queues(vi, vi->curr_queue_pairs);
> >> >> + if (ret)
> >> >> + goto err;
> >> >> +
> >> >> + virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> >> >> + virtio_config_enable(dev);
> >> >> + return 0;
> >> >> +
> >> >> +err:
> >> >> + virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
> >> >> + return ret;
> >> >> +}
> >> >> +
> >> >> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
> >> >> {
> >> >> struct scatterlist sg;
> >> >
> >> > I have a question here though. How do things like MAC address
> >> > get restored?
> >> >
> >> > What about the rx mode?
> >> >
> >> > vlans?
> >>
> >> The function as is releases and reinitializes only ring state.
> >> Device configuration such as mac and vlan persist across
> >> the reset.
> >
> > What gave you this impression? Take a look at e.g. this
> > code in qemu:
> >
> > static void virtio_net_reset(VirtIODevice *vdev)
> > {
> > VirtIONet *n = VIRTIO_NET(vdev);
> >
> > /* Reset back to compatibility mode */
> > n->promisc = 1;
> > n->allmulti = 0;
> > n->alluni = 0;
> > n->nomulti = 0;
> > n->nouni = 0;
> > n->nobcast = 0;
> > /* multiqueue is disabled by default */
> > n->curr_queues = 1;
> > timer_del(n->announce_timer);
> > n->announce_counter = 0;
> > n->status &= ~VIRTIO_NET_S_ANNOUNCE;
> >
> > /* Flush any MAC and VLAN filter table state */
> > n->mac_table.in_use = 0;
> > n->mac_table.first_multi = 0;
> > n->mac_table.multi_overflow = 0;
> > n->mac_table.uni_overflow = 0;
> > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
> > memcpy(&n->mac[0], &n->nic->conf->macaddr, sizeof(n->mac));
> > qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> > memset(n->vlans, 0, MAX_VLAN >> 3);
> > }
> >
> > So device seems to lose all state, you have to re-program it.
>
> Oh, indeed! The guest does not reset its state, so it might
> be out of sync with the host after the operation. Was this not
> an issue when previously resetting in the context of xdp?
I suspect it was broken back then, too.
> >> > Also, it seems that LINK_ANNOUNCE requests will get ignored
> >> > even if they got set before the reset, leading to downtime.
> >>
> >> Do you mean act on VIRTIO_NET_F_GUEST_ANNOUNCE
> >> requests? That flag is tested and netdev_notify_peers
> >> called before resetting virtio ring state.
> >
> > Yes but I wonder if there's a race where announce
> > is set after it is read but before NEED_RESET is read.
> >
> > Re-reading status from the config before reset
> > might be necessary.
>
> Thanks, I'll have a look. Perhaps a host should simply not
> request a reset while it is waiting for an announce ack.
It's one option though we can't make this change for existing hosts.
We also have the reverse condition where announce is requested after
NEED_RESET is set.
--
MST
^ permalink raw reply
* [PATCH v2 net-next 5/5] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1508171814-16713-1-git-send-email-dsahern@gmail.com>
Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
to set an error message on RIF or VR overflow. Now on overflow of
either resource the user gets an informative message as opposed to
failing with EBUSY.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 114 +++++++++++++--------
1 file changed, 69 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 2a7f066dfab5..9e0b46513ca7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
}
static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
- u32 tb_id)
+ u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
int err;
vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
- if (!vr)
+ if (!vr) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported virtual routers");
return ERR_PTR(-EBUSY);
+ }
vr->fib4 = mlxsw_sp_fib_create(vr, MLXSW_SP_L3_PROTO_IPV4);
if (IS_ERR(vr->fib4))
return ERR_CAST(vr->fib4);
@@ -775,14 +778,15 @@ static void mlxsw_sp_vr_destroy(struct mlxsw_sp_vr *vr)
vr->fib4 = NULL;
}
-static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id)
+static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
tb_id = mlxsw_sp_fix_tb_id(tb_id);
vr = mlxsw_sp_vr_find(mlxsw_sp, tb_id);
if (!vr)
- vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id, extack);
return vr;
}
@@ -948,7 +952,8 @@ static u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params);
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack);
static struct mlxsw_sp_rif_ipip_lb *
mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
@@ -966,7 +971,7 @@ mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
.lb_config = ipip_ops->ol_loopback_config(mlxsw_sp, ol_dev),
};
- rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common, NULL);
if (IS_ERR(rif))
return ERR_CAST(rif);
return container_of(rif, struct mlxsw_sp_rif_ipip_lb, common);
@@ -3711,7 +3716,7 @@ mlxsw_sp_fib_node_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id, const void *addr,
struct mlxsw_sp_vr *vr;
int err;
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id, NULL);
if (IS_ERR(vr))
return ERR_CAST(vr);
fib = mlxsw_sp_vr_fib(vr, proto);
@@ -4750,7 +4755,7 @@ static int mlxsw_sp_router_fibmr_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -4783,7 +4788,7 @@ mlxsw_sp_router_fibmr_vif_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -5346,7 +5351,8 @@ const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params)
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack)
{
u32 tb_id = l3mdev_fib_table(params->dev);
const struct mlxsw_sp_rif_ops *ops;
@@ -5360,14 +5366,16 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
type = mlxsw_sp_dev_rif_type(mlxsw_sp, params->dev);
ops = mlxsw_sp->router->rif_ops_arr[type];
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN, extack);
if (IS_ERR(vr))
return ERR_CAST(vr);
vr->rif_count++;
err = mlxsw_sp_rif_index_alloc(mlxsw_sp, &rif_index);
- if (err)
+ if (err) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported router interfaces");
goto err_rif_index_alloc;
+ }
rif = mlxsw_sp_rif_alloc(ops->rif_size, rif_index, vr->id, params->dev);
if (!rif) {
@@ -5454,7 +5462,8 @@ mlxsw_sp_rif_subport_params_init(struct mlxsw_sp_rif_params *params,
static int
mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp_port_vlan->mlxsw_sp_port;
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
@@ -5470,7 +5479,7 @@ mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
};
mlxsw_sp_rif_subport_params_init(¶ms, mlxsw_sp_port_vlan);
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
}
@@ -5525,7 +5534,8 @@ mlxsw_sp_port_vlan_router_leave(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
struct net_device *port_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(port_dev);
struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan;
@@ -5537,7 +5547,7 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
return mlxsw_sp_port_vlan_router_join(mlxsw_sp_port_vlan,
- l3_dev);
+ l3_dev, extack);
case NETDEV_DOWN:
mlxsw_sp_port_vlan_router_leave(mlxsw_sp_port_vlan);
break;
@@ -5547,19 +5557,22 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_port_event(struct net_device *port_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(port_dev) ||
netif_is_lag_port(port_dev) ||
netif_is_ovs_port(port_dev))
return 0;
- return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1);
+ return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1,
+ extack);
}
static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
struct net_device *lag_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct net_device *port_dev;
struct list_head *iter;
@@ -5569,7 +5582,8 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
if (mlxsw_sp_port_dev_check(port_dev)) {
err = mlxsw_sp_inetaddr_port_vlan_event(l3_dev,
port_dev,
- event, vid);
+ event, vid,
+ extack);
if (err)
return err;
}
@@ -5579,16 +5593,19 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_lag_event(struct net_device *lag_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(lag_dev))
return 0;
- return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1);
+ return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1,
+ extack);
}
static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_lower_get(l3_dev);
struct mlxsw_sp_rif_params params = {
@@ -5598,7 +5615,7 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
break;
@@ -5612,7 +5629,8 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct net_device *real_dev = vlan_dev_real_dev(vlan_dev);
u16 vid = vlan_dev_vlan_id(vlan_dev);
@@ -5622,27 +5640,28 @@ static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
if (mlxsw_sp_port_dev_check(real_dev))
return mlxsw_sp_inetaddr_port_vlan_event(vlan_dev, real_dev,
- event, vid);
+ event, vid, extack);
else if (netif_is_lag_master(real_dev))
return __mlxsw_sp_inetaddr_lag_event(vlan_dev, real_dev, event,
- vid);
+ vid, extack);
else if (netif_is_bridge_master(real_dev) && br_vlan_enabled(real_dev))
- return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event, extack);
return 0;
}
static int __mlxsw_sp_inetaddr_event(struct net_device *dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (mlxsw_sp_port_dev_check(dev))
- return mlxsw_sp_inetaddr_port_event(dev, event);
+ return mlxsw_sp_inetaddr_port_event(dev, event, extack);
else if (netif_is_lag_master(dev))
- return mlxsw_sp_inetaddr_lag_event(dev, event);
+ return mlxsw_sp_inetaddr_lag_event(dev, event, extack);
else if (netif_is_bridge_master(dev))
- return mlxsw_sp_inetaddr_bridge_event(dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(dev, event, extack);
else if (is_vlan_dev(dev))
- return mlxsw_sp_inetaddr_vlan_event(dev, event);
+ return mlxsw_sp_inetaddr_vlan_event(dev, event, extack);
else
return 0;
}
@@ -5668,7 +5687,7 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
return notifier_from_errno(err);
}
@@ -5690,7 +5709,7 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, ivi->extack);
out:
return notifier_from_errno(err);
}
@@ -5719,7 +5738,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- __mlxsw_sp_inetaddr_event(dev, event);
+ __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
rtnl_unlock();
dev_put(dev);
@@ -5771,7 +5790,7 @@ int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, i6vi->extack);
out:
return notifier_from_errno(err);
}
@@ -5848,7 +5867,8 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev)
}
static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_rif *rif;
@@ -5857,9 +5877,9 @@ static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
*/
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (rif)
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, extack);
- return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP);
+ return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP, extack);
}
static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
@@ -5870,7 +5890,7 @@ static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (!rif)
return;
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, NULL);
}
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
@@ -5886,10 +5906,14 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
case NETDEV_PRECHANGEUPPER:
return 0;
case NETDEV_CHANGEUPPER:
- if (info->linking)
- err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev);
- else
+ if (info->linking) {
+ struct netlink_ext_ack *extack;
+
+ extack = netdev_notifier_info_to_extack(&info->info);
+ err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev, extack);
+ } else {
mlxsw_sp_port_vrf_leave(mlxsw_sp, l3_dev);
+ }
break;
}
@@ -6196,7 +6220,7 @@ mlxsw_sp_rif_ipip_lb_configure(struct mlxsw_sp_rif *rif)
struct mlxsw_sp_vr *ul_vr;
int err;
- ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id);
+ ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id, NULL);
if (IS_ERR(ul_vr))
return PTR_ERR(ul_vr);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] can: check for null sk before deferencing it via the call to sock_net
From: Josh Boyer @ 2017-10-16 16:37 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Colin King, Marc Kleine-Budde, David S . Miller, linux-can,
netdev, kernel-janitors, Linux-Kernel@Vger. Kernel. Org
In-Reply-To: <f5ce1047-eb37-151a-c7b7-9b47b70789f7@hartkopp.net>
On Fri, Sep 8, 2017 at 1:46 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
>
> On 09/08/2017 05:02 PM, Colin King wrote:
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The assignment of net via call sock_net will dereference sk. This
>> is performed before a sanity null check on sk, so there could be
>> a potential null dereference on the sock_net call if sk is null.
>> Fix this by assigning net after the sk null check. Also replace
>> the sk == NULL with the more usual !sk idiom.
>>
>> Detected by CoverityScan CID#1431862 ("Dereference before null check")
>>
>> Fixes: 384317ef4187 ("can: network namespace support for CAN_BCM
>> protocol")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
I don't see this one queued up in the net or net-next trees. Did it
fall through the cracks or did it get queued up elsewhere? Seems like
it's a good candidate to get into 4.14?
josh
>
>
> Thanks Collin!
>
>
>> ---
>> net/can/bcm.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/can/bcm.c b/net/can/bcm.c
>> index 47a8748d953a..a3791674b8ce 100644
>> --- a/net/can/bcm.c
>> +++ b/net/can/bcm.c
>> @@ -1493,13 +1493,14 @@ static int bcm_init(struct sock *sk)
>> static int bcm_release(struct socket *sock)
>> {
>> struct sock *sk = sock->sk;
>> - struct net *net = sock_net(sk);
>> + struct net *net;
>> struct bcm_sock *bo;
>> struct bcm_op *op, *next;
>> - if (sk == NULL)
>> + if (!sk)
>> return 0;
>> + net = sock_net(sk);
>> bo = bcm_sk(sk);
>> /* remove bcm_ops, timer, rx_unregister(), etc. */
>>
>
^ permalink raw reply
* [PATCH v2 net-next 4/5] mlxsw: spectrum: router: Add support for address validator notifier
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1508171814-16713-1-git-send-email-dsahern@gmail.com>
Add support for inetaddr_validator and inet6addr_validator. The
notifiers provide a means for validating ipv4 and ipv6 addresses
before the addresses are installed and on failure the error
is propagated back to the user.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 15 ++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 52 ++++++++++++++++++++++
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 321988ac57cc..d51402f98f97 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4505,9 +4505,16 @@ static struct notifier_block mlxsw_sp_netdevice_nb __read_mostly = {
.notifier_call = mlxsw_sp_netdevice_event,
};
+static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inetaddr_valid_event,
+};
+
static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
.notifier_call = mlxsw_sp_inetaddr_event,
- .priority = 10, /* Must be called before FIB notifier block */
+};
+
+static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inet6addr_valid_event,
};
static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
@@ -4533,7 +4540,9 @@ static int __init mlxsw_sp_module_init(void)
int err;
register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
+ register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
@@ -4552,7 +4561,9 @@ static int __init mlxsw_sp_module_init(void)
err_core_driver_register:
unregister_netevent_notifier(&mlxsw_sp_router_netevent_nb);
unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
+ unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
unregister_netdevice_notifier(&mlxsw_sp_netdevice_nb);
return err;
}
@@ -4563,7 +4574,9 @@ static void __exit mlxsw_sp_module_exit(void)
mlxsw_core_driver_unregister(&mlxsw_sp_driver);
unregister_netevent_notifier(&mlxsw_sp_router_netevent_nb);
unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
+ unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
unregister_netdevice_notifier(&mlxsw_sp_netdevice_nb);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 8e45183dc9bb..4865a6f58c83 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
struct netdev_notifier_changeupper_info *info);
void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 6a356f4b99a3..2a7f066dfab5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5656,6 +5656,32 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
struct mlxsw_sp_rif *rif;
int err = 0;
+ /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
+ if (event == NETDEV_UP)
+ goto out;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in_validator_info *ivi = (struct in_validator_info *) ptr;
+ struct net_device *dev = ivi->ivi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
goto out;
@@ -5708,6 +5734,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
struct net_device *dev = if6->idev->dev;
+ /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
+ if (event == NETDEV_UP)
+ return NOTIFY_DONE;
+
if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
return NOTIFY_DONE;
@@ -5724,6 +5754,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
return NOTIFY_DONE;
}
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in6_validator_info *i6vi = (struct in6_validator_info *) ptr;
+ struct net_device *dev = i6vi->i6vi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
const char *mac, int mtu)
{
--
2.1.4
^ permalink raw reply related
* [PATCH v2 net-next 3/5] net: Add extack to validator_info structs used for address notifier
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1508171814-16713-1-git-send-email-dsahern@gmail.com>
Add extack to in_validator_info and in6_validator_info. Update the one
user of each, ipvlan, to return an error message for failures.
Only manual configuration of an address is plumbed in the IPv6 code path.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ipvlan/ipvlan_main.c | 10 ++++++++--
include/linux/inetdevice.h | 1 +
include/net/addrconf.h | 1 +
net/ipv4/devinet.c | 8 +++++---
net/ipv6/addrconf.c | 22 ++++++++++++----------
5 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 6842739b6679..f0ab55df57f1 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -847,8 +847,11 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
+ if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
+ NL_SET_ERR_MSG(i6vi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
@@ -917,8 +920,11 @@ static int ipvlan_addr4_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
+ if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
+ NL_SET_ERR_MSG(ivi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 751d051f0bc7..681dff30940b 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -154,6 +154,7 @@ struct in_ifaddr {
struct in_validator_info {
__be32 ivi_addr;
struct in_device *ivi_dev;
+ struct netlink_ext_ack *extack;
};
int register_inetaddr_notifier(struct notifier_block *nb);
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 87981cd63180..b8b16437c6d5 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -55,6 +55,7 @@ struct prefix_info {
struct in6_validator_info {
struct in6_addr i6vi_addr;
struct inet6_dev *i6vi_dev;
+ struct netlink_ext_ack *extack;
};
#define IN6_ADDR_HSIZE_SHIFT 4
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7ce22a2c07ce..93773e5a80c7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -444,7 +444,7 @@ static void check_lifetime(struct work_struct *work);
static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
- u32 portid)
+ u32 portid, struct netlink_ext_ack *extack)
{
struct in_device *in_dev = ifa->ifa_dev;
struct in_ifaddr *ifa1, **ifap, **last_primary;
@@ -489,6 +489,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
*/
ivi.ivi_addr = ifa->ifa_address;
ivi.ivi_dev = ifa->ifa_dev;
+ ivi.extack = extack;
ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
NETDEV_UP, &ivi);
ret = notifier_to_errno(ret);
@@ -521,7 +522,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
static int inet_insert_ifa(struct in_ifaddr *ifa)
{
- return __inet_insert_ifa(ifa, NULL, 0);
+ return __inet_insert_ifa(ifa, NULL, 0, NULL);
}
static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
@@ -902,7 +903,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
return ret;
}
}
- return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid);
+ return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
+ extack);
} else {
inet_free_ifa(ifa);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index dd9c0c435f71..93f9c0a61911 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -987,7 +987,7 @@ static struct inet6_ifaddr *
ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
const struct in6_addr *peer_addr, int pfxlen,
int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
- bool can_block)
+ bool can_block, struct netlink_ext_ack *extack)
{
gfp_t gfp_flags = can_block ? GFP_KERNEL : GFP_ATOMIC;
struct net *net = dev_net(idev->dev);
@@ -1019,6 +1019,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct in6_validator_info i6vi = {
.i6vi_addr = *addr,
.i6vi_dev = idev,
+ .extack = extack,
};
err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
@@ -1356,7 +1357,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft, true);
+ tmp_valid_lft, tmp_prefered_lft, true, NULL);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2040,7 +2041,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
scope, flags, valid_lft,
- preferred_lft, false);
+ preferred_lft, false, NULL);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2498,7 +2499,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
pinfo->prefix_len,
addr_type&IPV6_ADDR_SCOPE_MASK,
addr_flags, valid_lft,
- prefered_lft, false);
+ prefered_lft, false, NULL);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2808,7 +2809,8 @@ static int inet6_addr_add(struct net *net, int ifindex,
const struct in6_addr *pfx,
const struct in6_addr *peer_pfx,
unsigned int plen, __u32 ifa_flags,
- __u32 prefered_lft, __u32 valid_lft)
+ __u32 prefered_lft, __u32 valid_lft,
+ struct netlink_ext_ack *extack)
{
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
@@ -2867,7 +2869,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
}
ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft, true);
+ valid_lft, prefered_lft, true, extack);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
@@ -2952,7 +2954,7 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
rtnl_lock();
err = inet6_addr_add(net, ireq.ifr6_ifindex, &ireq.ifr6_addr, NULL,
ireq.ifr6_prefixlen, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
rtnl_unlock();
return err;
}
@@ -2983,7 +2985,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifp = ipv6_add_addr(idev, addr, NULL, plen,
scope, IFA_F_PERMANENT,
INFINITY_LIFE_TIME, INFINITY_LIFE_TIME,
- true);
+ true, NULL);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3083,7 +3085,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
#endif
ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true, NULL);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp);
@@ -4586,7 +4588,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
*/
return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
ifm->ifa_prefixlen, ifa_flags,
- preferred_lft, valid_lft);
+ preferred_lft, valid_lft, extack);
}
if (nlh->nlmsg_flags & NLM_F_EXCL ||
--
2.1.4
^ permalink raw reply related
* [PATCH v2 net-next 2/5] net: ipv6: Make inet6addr_validator a blocking notifier
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1508171814-16713-1-git-send-email-dsahern@gmail.com>
inet6addr_validator chain was added by commit 3ad7d2468f79f ("Ipvlan
should return an error when an address is already in use") to allow
address validation before changes are committed and to be able to
fail the address change with an error back to the user. The address
validation is not done for addresses received from router
advertisements.
Handling RAs in softirq context is the only reason for the notifier
chain to be atomic versus blocking. Since the only current user, ipvlan,
of the validator chain ignores softirq context, the notifier can be made
blocking and simply not invoked for softirq path.
The blocking option is needed by spectrum for example to validate
resources for an adding an address to an interface.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ipvlan/ipvlan_main.c | 4 ----
net/ipv6/addrconf.c | 21 ++++++++++++++-------
net/ipv6/addrconf_core.c | 9 +++++----
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 3cf67db513e2..6842739b6679 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -808,10 +808,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
struct net_device *dev = (struct net_device *)if6->idev->dev;
struct ipvl_dev *ipvlan = netdev_priv(dev);
- /* FIXME IPv6 autoconf calls us from bh without RTNL */
- if (in_softirq())
- return NOTIFY_DONE;
-
if (!netif_is_ipvlan(dev))
return NOTIFY_DONE;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a8d202b1b919..dd9c0c435f71 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -993,7 +993,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
struct rt6_info *rt = NULL;
- struct in6_validator_info i6vi;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -1013,12 +1012,20 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
goto out;
}
- i6vi.i6vi_addr = *addr;
- i6vi.i6vi_dev = idev;
- err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
- err = notifier_to_errno(err);
- if (err < 0)
- goto out;
+ /* validator notifier needs to be blocking;
+ * do not call in atomic context
+ */
+ if (can_block) {
+ struct in6_validator_info i6vi = {
+ .i6vi_addr = *addr,
+ .i6vi_dev = idev,
+ };
+
+ err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
+ err = notifier_to_errno(err);
+ if (err < 0)
+ goto out;
+ }
ifa = kzalloc(sizeof(*ifa), gfp_flags);
if (!ifa) {
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index 9e3488d50b15..32b564dfd02a 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -88,7 +88,7 @@ int __ipv6_addr_type(const struct in6_addr *addr)
EXPORT_SYMBOL(__ipv6_addr_type);
static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
-static ATOMIC_NOTIFIER_HEAD(inet6addr_validator_chain);
+static BLOCKING_NOTIFIER_HEAD(inet6addr_validator_chain);
int register_inet6addr_notifier(struct notifier_block *nb)
{
@@ -110,19 +110,20 @@ EXPORT_SYMBOL(inet6addr_notifier_call_chain);
int register_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_register(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_register(&inet6addr_validator_chain, nb);
}
EXPORT_SYMBOL(register_inet6addr_validator_notifier);
int unregister_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_unregister(&inet6addr_validator_chain,
+ nb);
}
EXPORT_SYMBOL(unregister_inet6addr_validator_notifier);
int inet6addr_validator_notifier_call_chain(unsigned long val, void *v)
{
- return atomic_notifier_call_chain(&inet6addr_validator_chain, val, v);
+ return blocking_notifier_call_chain(&inet6addr_validator_chain, val, v);
}
EXPORT_SYMBOL(inet6addr_validator_notifier_call_chain);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 net-next 1/5] ipv6: addrconf: cleanup locking in ipv6_add_addr
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1508171814-16713-1-git-send-email-dsahern@gmail.com>
ipv6_add_addr is called in process context with rtnl lock held
(e.g., manual config of an address) or during softirq processing
(e.g., autoconf and address from a router advertisement).
Currently, ipv6_add_addr calls rcu_read_lock_bh shortly after entry
and does not call unlock until exit, minus the call around the address
validator notifier. Similarly, addrconf_hash_lock is taken after the
validator notifier and held until exit. This forces the allocation of
inet6_ifaddr to always be atomic.
Refactor ipv6_add_addr as follows:
1. add an input boolean to discriminate the call path (process context
or softirq). This new flag controls whether the alloc can be done
with GFP_KERNEL or GFP_ATOMIC.
2. Move the rcu_read_lock_bh and unlock calls only around functions that
do rcu updates.
3. Remove the in6_dev_hold and put added by 3ad7d2468f79f ("Ipvlan should
return an error when an address is already in use."). This was done
presumably because rcu_read_unlock_bh needs to be called before calling
the validator. Since rcu_read_lock is not needed before the validator
runs revert the hold and put added by 3ad7d2468f79f and only do the
hold when setting ifp->idev.
4. move duplicate address check and insertion of new address in the global
address hash into a helper. The helper is called after an ifa is
allocated and filled in.
This allows the ifa for manually configured addresses to be done with
GFP_KERNEL and reduces the overall amount of time with rcu_read_lock held
and hash table spinlock held.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 104 ++++++++++++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 44 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4603aa488f4f..a8d202b1b919 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -957,18 +957,43 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
return hash_32(ipv6_addr_hash(addr), IN6_ADDR_HSIZE_SHIFT);
}
+static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
+{
+ unsigned int hash;
+ int err = 0;
+
+ spin_lock(&addrconf_hash_lock);
+
+ /* Ignore adding duplicate addresses on an interface */
+ if (ipv6_chk_same_addr(dev_net(dev), &ifa->addr, dev)) {
+ ADBG("ipv6_add_addr: already assigned\n");
+ err = -EEXIST;
+ goto out;
+ }
+
+ /* Add to big hash table */
+ hash = inet6_addr_hash(&ifa->addr);
+ hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
+
+out:
+ spin_unlock(&addrconf_hash_lock);
+
+ return err;
+}
+
/* On success it returns ifp with increased reference count */
static struct inet6_ifaddr *
ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
const struct in6_addr *peer_addr, int pfxlen,
- int scope, u32 flags, u32 valid_lft, u32 prefered_lft)
+ int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
+ bool can_block)
{
+ gfp_t gfp_flags = can_block ? GFP_KERNEL : GFP_ATOMIC;
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
- struct rt6_info *rt;
+ struct rt6_info *rt = NULL;
struct in6_validator_info i6vi;
- unsigned int hash;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -978,42 +1003,24 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
addr_type & IPV6_ADDR_LOOPBACK))
return ERR_PTR(-EADDRNOTAVAIL);
- rcu_read_lock_bh();
-
- in6_dev_hold(idev);
-
if (idev->dead) {
err = -ENODEV; /*XXX*/
- goto out2;
+ goto out;
}
if (idev->cnf.disable_ipv6) {
err = -EACCES;
- goto out2;
+ goto out;
}
i6vi.i6vi_addr = *addr;
i6vi.i6vi_dev = idev;
- rcu_read_unlock_bh();
-
err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
-
- rcu_read_lock_bh();
err = notifier_to_errno(err);
- if (err)
- goto out2;
-
- spin_lock(&addrconf_hash_lock);
-
- /* Ignore adding duplicate addresses on an interface */
- if (ipv6_chk_same_addr(dev_net(idev->dev), addr, idev->dev)) {
- ADBG("ipv6_add_addr: already assigned\n");
- err = -EEXIST;
+ if (err < 0)
goto out;
- }
-
- ifa = kzalloc(sizeof(struct inet6_ifaddr), GFP_ATOMIC);
+ ifa = kzalloc(sizeof(*ifa), gfp_flags);
if (!ifa) {
ADBG("ipv6_add_addr: malloc failed\n");
err = -ENOBUFS;
@@ -1023,6 +1030,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
rt = addrconf_dst_alloc(idev, addr, false);
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
+ rt = NULL;
goto out;
}
@@ -1053,16 +1061,21 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifa->rt = rt;
ifa->idev = idev;
+ in6_dev_hold(idev);
+
/* For caller */
refcount_set(&ifa->refcnt, 1);
- /* Add to big hash table */
- hash = inet6_addr_hash(addr);
+ rcu_read_lock_bh();
- hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
- spin_unlock(&addrconf_hash_lock);
+ err = ipv6_add_addr_hash(idev->dev, ifa);
+ if (err < 0) {
+ rcu_read_unlock_bh();
+ goto out;
+ }
write_lock(&idev->lock);
+
/* Add to inet6_dev unicast addr list. */
ipv6_link_dev_addr(idev, ifa);
@@ -1073,21 +1086,23 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
in6_ifa_hold(ifa);
write_unlock(&idev->lock);
-out2:
+
rcu_read_unlock_bh();
- if (likely(err == 0))
- inet6addr_notifier_call_chain(NETDEV_UP, ifa);
- else {
- kfree(ifa);
- in6_dev_put(idev);
+ inet6addr_notifier_call_chain(NETDEV_UP, ifa);
+out:
+ if (unlikely(err < 0)) {
+ if (rt)
+ ip6_rt_put(rt);
+ if (ifa) {
+ if (ifa->idev)
+ in6_dev_put(ifa->idev);
+ kfree(ifa);
+ }
ifa = ERR_PTR(err);
}
return ifa;
-out:
- spin_unlock(&addrconf_hash_lock);
- goto out2;
}
enum cleanup_prefix_rt_t {
@@ -1334,7 +1349,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft);
+ tmp_valid_lft, tmp_prefered_lft, true);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2018,7 +2033,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
scope, flags, valid_lft,
- preferred_lft);
+ preferred_lft, false);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2476,7 +2491,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
pinfo->prefix_len,
addr_type&IPV6_ADDR_SCOPE_MASK,
addr_flags, valid_lft,
- prefered_lft);
+ prefered_lft, false);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2845,7 +2860,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
}
ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft);
+ valid_lft, prefered_lft, true);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
@@ -2960,7 +2975,8 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifp = ipv6_add_addr(idev, addr, NULL, plen,
scope, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME,
+ true);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3060,7 +3076,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
#endif
ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp);
--
2.1.4
^ permalink raw reply related
* [PATCH v2 net-next 0/5] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow
From: David Ahern @ 2017-10-16 16:36 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
Currently, exceeding the number of VRF instances or the number of router
interfaces either fails with a non-intuitive EBUSY:
$ ip li set swp1s1.6 vrf vrf-1s1-6 up
RTNETLINK answers: Device or resource busy
or fails silently (IPv6) since the checks are done in a work queue. This
set adds support for the address validator notifier to spectrum which
allows ext-ack based messages to be returned on failure.
To make that happen the IPv6 version needs to be converted from atomic
to blocking (patch 2), and then support for extack needs to be added
to the notifier (patch 3). Patch 1 reworks the locking in ipv6_add_addr
to work better in the atomic and non-atomic code paths. Patches 4 and 5
add the validator notifier to spectrum and then plumb the extack argument
through spectrum_router.
With this set, VRF overflows fail with:
$ ip li set swp1s1.6 vrf vrf-1s1-6 up
Error: spectrum: Exceeded number of supported VRF.
and RIF overflows fail with:
$ ip addr add dev swp1s2.191 10.12.191.1/24
Error: spectrum: Exceeded number of supported router interfaces.
v1 -> v2
- fix error path in ipv6_add_addr: reset rt to NULL (Ido comment) and
add in6_dev_put on ifa once the hold has been done
RFC -> v1
- addressed various comments from Ido
- refactored ipv6_add_addr to allow ifa's to be allocated with
GFP_KERNEL as requested by DaveM
David Ahern (5):
ipv6: addrconf: cleanup locking in ipv6_add_addr
net: ipv6: Make inet6addr_validator a blocking notifier
net: Add extack to validator_info structs used for address notifier
mlxsw: spectrum: router: Add support for address validator notifier
mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 15 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 +
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 162 +++++++++++++++------
drivers/net/ipvlan/ipvlan_main.c | 14 +-
include/linux/inetdevice.h | 1 +
include/net/addrconf.h | 1 +
net/ipv4/devinet.c | 8 +-
net/ipv6/addrconf.c | 129 +++++++++-------
net/ipv6/addrconf_core.c | 9 +-
9 files changed, 234 insertions(+), 109 deletions(-)
--
2.1.4
^ permalink raw reply
* Re: [PATCH net] bpf: disallow arithmetic operations on context pointer
From: Jakub Kicinski @ 2017-10-16 16:30 UTC (permalink / raw)
To: Edward Cree; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <44a8fb95-298b-8715-ce4e-60df60c86343@solarflare.com>
On Mon, 16 Oct 2017 17:16:24 +0100, Edward Cree wrote:
> On 16/10/17 16:45, Jakub Kicinski wrote:
> > Commit f1174f77b50c ("bpf/verifier: rework value tracking")
> > removed the crafty selection of which pointer types are
> > allowed to be modified. This is OK for most pointer types
> > since adjust_ptr_min_max_vals() will catch operations on
> > immutable pointers. One exception is PTR_TO_CTX which is
> > now allowed to be offseted freely.
> >
> > The intent of aforementioned commit was to allow context
> > access via modified registers. The offset passed to
> > ->is_valid_access() verifier callback has been adjusted
> > by the value of the variable offset.
> >
> > What is missing, however, is taking the variable offset
> > into account when the context register is used. Or in terms
> > of the code adding the offset to the value passed to the
> > ->convert_ctx_access() callback. This leads to the following
> Good catch.
> So the problem is just that convert_ctx_access() can't deal
> with it, yes? Assuming that the offset is constant, because
> otherwise we'd reject it anyway, we _could_ stash that offset
> in insn_aux_data, and reject any paths that tried to change it
> subsequently; and then convert_ctx_access() could be given the
> total (off + reg->off) and then the reg->off could be subtracted
> from the result, giving the right 'converted' insn offset.
> That would then mean that your example,
> > eBPF user code:
> >
> > r1 += 68
> > r0 = *(u32 *)(r1 + 8)
> > exit
> >
> > being translated to this in kernel space:
> >
> > 0: (07) r1 += 68
> > 1: (61) r0 = *(u32 *)(r1 +180)
> > 2: (95) exit
> would instead convert offset 76 to something else, let's say
> 200 just for the sake of argument, and produce
> 1: (61) r0 = *(u32 *)(r1 + 132)
> which would then do the right thing at run time.
>
> However, I don't know whether anyone would actually want this
> to be supported for their programs, and so I'm happy to
> disallow this for net and then maybe we can follow up in
> net-next with the change I describe above if it's useful.
That was my thinking too. We would have to stash the offset and make
sure that it doesn't change on different paths, but it's probably not
suitable for -net, and nobody uses that today (although Alexei
mentioned there may be valid use cases, especially in tracing).
> > Offset 8 is corresponding to 180 in the kernel, but offset
> > 76 is valid too. Verifier will "accept" access to offset
> > 68+8=76 but then "convert" access to offset 8 as 180.
> > Effective access to offset 248 is beyond the kernel context.
> > (This is a __sk_buff example on a debug-heavy kernel -
> > packet mark is 8 -> 180, 76 would be data.)
> >
> > Dereferencing the modified context pointer is not as easy
> > as dereferencing other types, because we have to translate
> > the access to reading a field in kernel structures which is
> > usually at a different offset and often of a different size.
> > To allow modifying the pointer we would have to make sure
> > that given eBPF instruction will always access the same
> > field or the fields accessed are "compatible" in terms of
> > offset and size...
> >
> > Disallow dereferencing modified context pointers and add
> > to selftests the test case described here.
> >
> > Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > Dave, a merge note - in net-next this will need env to be passed
> > to verbose().
> >
> > kernel/bpf/verifier.c | 8 ++++++--
> > tools/testing/selftests/bpf/test_verifier.c | 14 ++++++++++++++
> > 2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 8b8d6ba39e23..8499759d0c7a 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -1116,7 +1116,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> > /* ctx accesses must be at a fixed offset, so that we can
> > * determine what type of data were returned.
> > */
> > - if (!tnum_is_const(reg->var_off)) {
> > + if (reg->off) {
> > + verbose("derefence of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
> This is slightly unclear, it's not that two adds is bad (e.g. r1 += 8;
> r0 = *(u32 *)r1 is bad too), it's that the offset must be in the load,
> not the register; your message might be accurate for some compilers but
> not in full generality (especially for assemblers without compiling).
I'm happy to hear better suggestions :) I've spent quite a bit of time
scratching my head thinking how to phrase this best. The first
part of the message is general enough IMHO, the second is targeted
mostly at C developers.
> Also, sp. "dereference".
Argh. aspell doesn't know that word, added to local dict now.
^ permalink raw reply
* RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: David Laight @ 2017-10-16 16:30 UTC (permalink / raw)
To: 'Andrew Lunn', Rodney Cummings
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli
In-Reply-To: <20171016160941.GA13339@lunn.ch>
From: Andrew Lunn
> Sent: 16 October 2017 17:10
...
> So, received Pause frames never leave the MAC. They don't get bridged,
> nor do they get passed up for host processing. They are purely point
> to point between two MAC peers. The destination is unambiguous. It is
> simple the other MAC peer. The destination address makes it clear it
> is a pause frame, the the source address seems to be unneeded.
>
> In this context, a random MAC addresses are safe.
Is there any reason why a fixed value (say 00:00:00:00:00:00)
can't be used?
> In the more general case, i would agree with you. Collisions are
> possible, causing problems.
For IP MAC addresses only go as far as the first router.
So the duplicates would (typically) have to be within the same subnet.
This makes the chance of a duplicate random address unlikely.
(Unless you have an un-subnetted class A network consisting of
multiple 1km long coax segments connected by 1km long repeaters
making a single collision domain.)
David
^ permalink raw reply
* RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
From: Rodney Cummings @ 2017-10-16 16:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,
David S. Miller, Florian Fainelli, David Laight
In-Reply-To: <20171016160941.GA13339@lunn.ch>
Hi Andrew,
I may have misunderstood.
If this MAC address is the destination, a local random MAC address doesn't work. PAUSE is using one of the 802.1Q reserved destination MAC addresses (as does LLDP, MSTP, 1588, etc). PAUSE needs to use that specially assigned address (01-80-C2-00-00-01).
I assumed we were talking about the source MAC address. For the source MAC address, I would agree that a local random MAC address works in some narrow scenarios.
My concern is that for a source MAC address, a local random MAC address is not safe in all networks, because it has the potential for duplication. That topic has been discussed quite a bit in IEEE 802.
Rodney
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, October 16, 2017 11:10 AM
> To: Rodney Cummings <rodney.cummings@ni.com>
> Cc: Vivien Didelot <vivien.didelot@savoirfairelinux.com>;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> kernel@savoirfairelinux.com; David S. Miller <davem@davemloft.net>;
> Florian Fainelli <f.fainelli@gmail.com>; David Laight
> <David.Laight@ACULAB.COM>
> Subject: Re: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
>
> On Mon, Oct 16, 2017 at 03:23:42PM +0000, Rodney Cummings wrote:
> > I am concerned about this proposed change.
> >
> > According to IEEE Std 802.1Q, a "higher layer entity" (i.e. end
> > station) that is internal to the bridge (i.e. switch) can use the
> > same individual global MAC address that the switch is using. That
> > behavior is common.
> >
> > Use of a local semi-random MAC address is dangerous on
> > Ethernet. Local MAC addresses can only be safely used in very narrow
> > use cases, most of which are Wi-Fi. Generally speaking, local MAC
> > addresses can be duplicated in the network, which causes many
> > Ethernet protocols to break down. Therefore, general-purpose
> > implementations like DSA cannot use a local MAC address in a
> > reliable manner.
>
> Hi Rodney
>
> This is interesting. So i did some research. The Marvell Switch only
> uses this MAC address for Pause frames. Nothing else.
>
> 802.3-2015 Section 2, Annex 31B says:
>
> The globally assigned 48-bit multicast address 01-80-C2-00-00-01 has
> been reserved for use in MAC Control PAUSE frames for inhibiting
> transmission of data frames from a DTE in a full duplex mode IEEE
> 802.3 LAN. IEEE 802.1D-conformant bridges will not forward frames
> sent to this multicast destination address, regardless of the state
> of the bridge’s ports, or whether or not the bridge
> implements the MAC Control sublayer. To allow generic full duplex
> flow control, stations implementing the PAUSE operation shall
> instruct the MAC (e.g., through layer management) to enable
> reception of frames with destination address equal to this multicast
> address.
>
> NOTE—By definition, an IEEE 802.3 LAN operating in full
> duplex mode comprises exactly two stations, thus there is no
> ambiguity regarding the destination DTE’s identity. The use
> of a well-known multicast address relieves the MAC Control sublayer
> and its client from having to know, and maintain knowledge of, the
> individual 48-bit address of the other DTE in a full duplex
> environment.
>
> When MAC Control PFC operation (see Annex 31D and IEEE Std 802.1Q)
> has been enabled, MAC Control PAUSE operation shall be disabled.
>
> So, received Pause frames never leave the MAC. They don't get bridged,
> nor do they get passed up for host processing. They are purely point
> to point between two MAC peers. The destination is unambiguous. It is
> simple the other MAC peer. The destination address makes it clear it
> is a pause frame, the the source address seems to be unneeded.
>
> In this context, a random MAC addresses are safe.
>
> In the more general case, i would agree with you. Collisions are
> possible, causing problems.
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next v5 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: Stephen Smalley @ 2017-10-16 16:34 UTC (permalink / raw)
To: Chenbo Feng, netdev
Cc: Jeffrey Vander Stoep, Alexei Starovoitov, lorenzo,
Daniel Borkmann, James Morris, Paul Moore, Chenbo Feng
In-Reply-To: <20171012205510.36028-6-chenbofeng.kernel@gmail.com>
On Thu, 2017-10-12 at 13:55 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce a bpf object related check when sending and receiving files
> through unix domain socket as well as binder. It checks if the
> receiving
> process have privilege to read/write the bpf map or use the bpf
> program.
> This check is necessary because the bpf maps and programs are using a
> anonymous inode as their shared inode so the normal way of checking
> the
> files and sockets when passing between processes cannot work properly
> on
> eBPF object. This check only works when the BPF_SYSCALL is
> configured.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
> include/linux/bpf.h | 3 +++
> kernel/bpf/syscall.c | 4 ++--
> security/selinux/hooks.c | 49
> ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 225740688ab7..81d6c01b8825 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -285,6 +285,9 @@ int bpf_prog_array_copy_to_user(struct
> bpf_prog_array __rcu *progs,
> #ifdef CONFIG_BPF_SYSCALL
> DECLARE_PER_CPU(int, bpf_prog_active);
>
> +extern const struct file_operations bpf_map_fops;
> +extern const struct file_operations bpf_prog_fops;
> +
> #define BPF_PROG_TYPE(_id, _ops) \
> extern const struct bpf_verifier_ops _ops;
> #define BPF_MAP_TYPE(_id, _ops) \
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index d3e152e282d8..8bdb98aa7f34 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -313,7 +313,7 @@ static ssize_t bpf_dummy_write(struct file *filp,
> const char __user *buf,
> return -EINVAL;
> }
>
> -static const struct file_operations bpf_map_fops = {
> +const struct file_operations bpf_map_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = bpf_map_show_fdinfo,
> #endif
> @@ -967,7 +967,7 @@ static void bpf_prog_show_fdinfo(struct seq_file
> *m, struct file *filp)
> }
> #endif
>
> -static const struct file_operations bpf_prog_fops = {
> +const struct file_operations bpf_prog_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = bpf_prog_show_fdinfo,
> #endif
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 12cf7de8cbed..ef7e5c1de640 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1815,6 +1815,10 @@ static inline int file_path_has_perm(const
> struct cred *cred,
> return inode_has_perm(cred, file_inode(file), av, &ad);
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +static int bpf_fd_pass(struct file *file, u32 sid);
> +#endif
> +
> /* Check whether a task can use an open file descriptor to
> access an inode in a given way. Check access to the
> descriptor itself, and then use dentry_has_perm to
> @@ -1845,6 +1849,12 @@ static int file_has_perm(const struct cred
> *cred,
> goto out;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> + rc = bpf_fd_pass(file, cred_sid(cred));
> + if (rc)
> + return rc;
> +#endif
> +
> /* av is zero if only checking access to the descriptor. */
> rc = 0;
> if (av)
> @@ -2165,6 +2175,12 @@ static int selinux_binder_transfer_file(struct
> task_struct *from,
> return rc;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> + rc = bpf_fd_pass(file, sid);
> + if (rc)
> + return rc;
> +#endif
> +
> if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> return 0;
>
> @@ -6288,6 +6304,39 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode)
> return av;
> }
>
> +/* This function will check the file pass through unix socket or
> binder to see
> + * if it is a bpf related object. And apply correspinding checks on
> the bpf
> + * object based on the type. The bpf maps and programs, not like
> other files and
> + * socket, are using a shared anonymous inode inside the kernel as
> their inode.
> + * So checking that inode cannot identify if the process have
> privilege to
> + * access the bpf object and that's why we have to add this
> additional check in
> + * selinux_file_receive and selinux_binder_transfer_files.
> + */
> +static int bpf_fd_pass(struct file *file, u32 sid)
> +{
> + struct bpf_security_struct *bpfsec;
> + struct bpf_prog *prog;
> + struct bpf_map *map;
> + int ret;
> +
> + if (file->f_op == &bpf_map_fops) {
> + map = file->private_data;
> + bpfsec = map->security;
> + ret = avc_has_perm(sid, bpfsec->sid,
> SECCLASS_BPF_MAP,
> + bpf_map_fmode_to_av(file-
> >f_mode), NULL);
> + if (ret)
> + return ret;
> + } else if (file->f_op == &bpf_prog_fops) {
> + prog = file->private_data;
> + bpfsec = prog->aux->security;
> + ret = avc_has_perm(sid, bpfsec->sid,
> SECCLASS_BPF_PROG,
> + BPF__PROG_RUN, NULL);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
My apologies, I should have caught this earlier, but you didn't update
the class/permission symbols used above when you re-spun patch 4/5 with
a single bpf class with distinct permissions defined.
Please, ensure that the entire series compiles and works before
sending.
Also, you likely should be cc'ing linux-security-module mailing list
and selinux mailing list; I thought you were doing so on earlier
versions.
> +
> static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
> {
> u32 sid = current_sid();
^ permalink raw reply
* Re: [PATCH iproute2 net-next] ip: bridge_slave: add support for per-port group_fwd_mask
From: Stephen Hemminger @ 2017-10-16 16:27 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa
In-Reply-To: <1507900373-9911-1-git-send-email-nikolay@cumulusnetworks.com>
On Fri, 13 Oct 2017 16:12:53 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> This patch adds the iproute2 support for getting and setting the
> per-port group_fwd_mask. It also tries to resolve the value into a more
> human friendly format by printing the known protocols instead of only
> the raw value.
> The man page is also updated with the new option.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied. Thanks.
^ permalink raw reply
* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Paweł Staszewski @ 2017-10-16 16:26 UTC (permalink / raw)
To: Pavlos Parissis, Alexander Duyck
Cc: Anders K. Pedersen | Cohaesio, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, alexander.h.duyck@intel.com
In-Reply-To: <b4995fc8-1420-fe96-26f7-ad5d986b7d39@gmail.com>
W dniu 2017-10-16 o 13:20, Pavlos Parissis pisze:
> On 15/10/2017 02:58 πμ, Alexander Duyck wrote:
>> Hi Pawel,
>>
>> To clarify is that Dave Miller's tree or Linus's that you are talking
>> about? If it is Dave's tree how long ago was it you pulled it since I
>> think the fix was just pushed by Jeff Kirsher a few days ago.
>>
>> The issue should be fixed in the following commit:
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/drivers/net/ethernet/intel/i40e/i40e_txrx.c?id=2b9478ffc550f17c6cd8c69057234e91150f5972
>>
>
> Do you know when it is going to be available on net-next and linux-stable repos?
>
> Cheers,
> Pavlos
>
>
I will make some tests today night with "net" git tree where this patch
is included.
Starting from 0:00 CET
:)
^ permalink raw reply
* Re: [patch net v2 4/4] selftests: Introduce a new test case to tc testsuite
From: Lucas Bates @ 2017-10-16 16:25 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko, davem
In-Reply-To: <1508152718-28726-5-git-send-email-chrism@mellanox.com>
On Mon, Oct 16, 2017 at 7:18 AM, Chris Mi <chrism@mellanox.com> wrote:
> In this patchset, we fixed a tc bug. This patch adds the test case
> that reproduces the bug. To run this test case, user should specify
> an existing NIC device:
> # sudo ./tdc.py -d enp4s0f0
>
> This test case belongs to category "flower". If user doesn't specify
> a NIC device, the test cases belong to "flower" will not be run.
>
> In this test case, we create 1M filters and all filters share the same
> action. When destroying all filters, kernel should not panic. It takes
> about 18s to run it.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
I'm a little wary about adding changes like these into tdc.py
directly; I don't think it's going to be sustainable in the long run.
Even the namespace creation I put in to the original version is too
specific and limiting.
There are some upcoming changes to tdc to help address these
particular issues. I'll ack this for now, thanks.
Acked-by: Lucas Bates <lucasb@mojatatu.com>
> ---
> .../tc-testing/tc-tests/filters/tests.json | 23 +++++++++++++++++++++-
> tools/testing/selftests/tc-testing/tdc.py | 20 +++++++++++++++----
> tools/testing/selftests/tc-testing/tdc_config.py | 2 ++
> 3 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
> index c727b96..5fa02d8 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
> @@ -17,5 +17,26 @@
> "teardown": [
> "$TC qdisc del dev $DEV1 ingress"
> ]
> + },
> + {
> + "id": "d052",
> + "name": "Add 1M filters with the same action",
> + "category": [
> + "filter",
> + "flower"
> + ],
> + "setup": [
> + "$TC qdisc add dev $DEV2 ingress",
> + "./tdc_batch.py $DEV2 $BATCH_FILE --share_action -n 1000000"
> + ],
> + "cmdUnderTest": "$TC -b $BATCH_FILE",
> + "expExitCode": "0",
> + "verifyCmd": "$TC actions list action gact",
> + "matchPattern": "action order 0: gact action drop.*index 1 ref 1000000 bind 1000000",
> + "matchCount": "1",
> + "teardown": [
> + "$TC qdisc del dev $DEV2 ingress",
> + "/bin/rm $BATCH_FILE"
> + ]
> }
> -]
> \ No newline at end of file
> +]
> diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
> index cd61b78..5f11f5d 100755
> --- a/tools/testing/selftests/tc-testing/tdc.py
> +++ b/tools/testing/selftests/tc-testing/tdc.py
> @@ -88,7 +88,7 @@ def prepare_env(cmdlist):
> exit(1)
>
>
> -def test_runner(filtered_tests):
> +def test_runner(filtered_tests, args):
> """
> Driver function for the unit tests.
>
> @@ -105,6 +105,8 @@ def test_runner(filtered_tests):
> for tidx in testlist:
> result = True
> tresult = ""
> + if "flower" in tidx["category"] and args.device == None:
> + continue
> print("Test " + tidx["id"] + ": " + tidx["name"])
> prepare_env(tidx["setup"])
> (p, procout) = exec_cmd(tidx["cmdUnderTest"])
> @@ -152,6 +154,10 @@ def ns_create():
> exec_cmd(cmd, False)
> cmd = 'ip -s $NS link set $DEV1 up'
> exec_cmd(cmd, False)
> + cmd = 'ip link set $DEV2 netns $NS'
> + exec_cmd(cmd, False)
> + cmd = 'ip -s $NS link set $DEV2 up'
> + exec_cmd(cmd, False)
>
>
> def ns_destroy():
> @@ -211,7 +217,8 @@ def set_args(parser):
> help='Execute the single test case with specified ID')
> parser.add_argument('-i', '--id', action='store_true', dest='gen_id',
> help='Generate ID numbers for new test cases')
> - return parser
> + parser.add_argument('-d', '--device',
> + help='Execute the test case in flower category')
> return parser
>
>
> @@ -225,6 +232,8 @@ def check_default_settings(args):
>
> if args.path != None:
> NAMES['TC'] = args.path
> + if args.device != None:
> + NAMES['DEV2'] = args.device
> if not os.path.isfile(NAMES['TC']):
> print("The specified tc path " + NAMES['TC'] + " does not exist.")
> exit(1)
> @@ -381,14 +390,17 @@ def set_operation_mode(args):
> if (len(alltests) == 0):
> print("Cannot find a test case with ID matching " + target_id)
> exit(1)
> - catresults = test_runner(alltests)
> + catresults = test_runner(alltests, args)
> print("All test results: " + "\n\n" + catresults)
> elif (len(target_category) > 0):
> + if (target_category == "flower") and args.device == None:
> + print("Please specify a NIC device (-d) to run category flower")
> + exit(1)
> if (target_category not in ucat):
> print("Specified category is not present in this file.")
> exit(1)
> else:
> - catresults = test_runner(testcases[target_category])
> + catresults = test_runner(testcases[target_category], args)
> print("Category " + target_category + "\n\n" + catresults)
>
> ns_destroy()
> diff --git a/tools/testing/selftests/tc-testing/tdc_config.py b/tools/testing/selftests/tc-testing/tdc_config.py
> index 0108737..b635251 100644
> --- a/tools/testing/selftests/tc-testing/tdc_config.py
> +++ b/tools/testing/selftests/tc-testing/tdc_config.py
> @@ -12,6 +12,8 @@ NAMES = {
> # Name of veth devices to be created for the namespace
> 'DEV0': 'v0p0',
> 'DEV1': 'v0p1',
> + 'DEV2': '',
> + 'BATCH_FILE': './batch.txt',
> # Name of the namespace to use
> 'NS': 'tcut'
> }
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [patch net v2 3/4] selftests: Introduce a new script to generate tc batch file
From: Lucas Bates @ 2017-10-16 16:24 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko, davem
In-Reply-To: <1508152718-28726-4-git-send-email-chrism@mellanox.com>
On Mon, Oct 16, 2017 at 7:18 AM, Chris Mi <chrism@mellanox.com> wrote:
> # ./tdc_batch.py -h
> usage: tdc_batch.py [-h] [-n NUMBER] [-o] [-s] [-p] device file
>
> TC batch file generator
>
> positional arguments:
> device device name
> file batch file name
>
> optional arguments:
> -h, --help show this help message and exit
> -n NUMBER, --number NUMBER
> how many lines in batch file
> -o, --skip_sw skip_sw (offload), by default skip_hw
> -s, --share_action all filters share the same action
> -p, --prio all filters have different prio
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Lucas Bates <lucasb@mojatatu.com>
> ---
> tools/testing/selftests/tc-testing/tdc_batch.py | 62 +++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100755 tools/testing/selftests/tc-testing/tdc_batch.py
>
> diff --git a/tools/testing/selftests/tc-testing/tdc_batch.py b/tools/testing/selftests/tc-testing/tdc_batch.py
> new file mode 100755
> index 0000000..707c6bf
> --- /dev/null
> +++ b/tools/testing/selftests/tc-testing/tdc_batch.py
> @@ -0,0 +1,62 @@
> +#!/usr/bin/python3
> +
> +"""
> +tdc_batch.py - a script to generate TC batch file
> +
> +Copyright (C) 2017 Chris Mi <chrism@mellanox.com>
> +"""
> +
> +import argparse
> +
> +parser = argparse.ArgumentParser(description='TC batch file generator')
> +parser.add_argument("device", help="device name")
> +parser.add_argument("file", help="batch file name")
> +parser.add_argument("-n", "--number", type=int,
> + help="how many lines in batch file")
> +parser.add_argument("-o", "--skip_sw",
> + help="skip_sw (offload), by default skip_hw",
> + action="store_true")
> +parser.add_argument("-s", "--share_action",
> + help="all filters share the same action",
> + action="store_true")
> +parser.add_argument("-p", "--prio",
> + help="all filters have different prio",
> + action="store_true")
> +args = parser.parse_args()
> +
> +device = args.device
> +file = open(args.file, 'w')
> +
> +number = 1
> +if args.number:
> + number = args.number
> +
> +skip = "skip_hw"
> +if args.skip_sw:
> + skip = "skip_sw"
> +
> +share_action = ""
> +if args.share_action:
> + share_action = "index 1"
> +
> +prio = "prio 1"
> +if args.prio:
> + prio = ""
> + if number > 0x4000:
> + number = 0x4000
> +
> +index = 0
> +for i in range(0x100):
> + for j in range(0x100):
> + for k in range(0x100):
> + mac = ("%02x:%02x:%02x" % (i, j, k))
> + src_mac = "e4:11:00:" + mac
> + dst_mac = "e4:12:00:" + mac
> + cmd = ("filter add dev %s %s protocol ip parent ffff: flower %s "
> + "src_mac %s dst_mac %s action drop %s" %
> + (device, prio, skip, src_mac, dst_mac, share_action))
> + file.write("%s\n" % cmd)
> + index += 1
> + if index >= number:
> + file.close()
> + exit(0)
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH iproute2 1/1] tests: Revert back /bin/sh in shebang
From: Stephen Hemminger @ 2017-10-16 16:25 UTC (permalink / raw)
To: Petr Vorel; +Cc: netdev
In-Reply-To: <20171015095945.30878-1-petr.vorel@gmail.com>
On Sun, 15 Oct 2017 11:59:45 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:
> This was added by mistake in commit ecd44e68
> ("tests: Remove bashisms (s/source/.)")
>
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
> testsuite/tests/ip/route/add_default_route.t | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testsuite/tests/ip/route/add_default_route.t b/testsuite/tests/ip/route/add_default_route.t
> index 0b566f1f..569ba1f8 100755
> --- a/testsuite/tests/ip/route/add_default_route.t
> +++ b/testsuite/tests/ip/route/add_default_route.t
> @@ -1,4 +1,4 @@
> -#!/bin/bash
> +#!/bin/sh
>
> . lib/generic.sh
>
Sure, applied
^ permalink raw reply
* Re: [PATCH iproute2 v2 0/4] Fix ip segfault when using --color switch
From: Stephen Hemminger @ 2017-10-16 16:24 UTC (permalink / raw)
To: Petr Vorel; +Cc: netdev, Julien Fortin
In-Reply-To: <20171013135719.18450-1-petr.vorel@gmail.com>
On Fri, 13 Oct 2017 15:57:15 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:
> Hi Stephen,
>
> I cleanup code not to use magic offsets. I kept it in separate commits,
> as it's clearer what happened.
>
> Petr Vorel (4):
> color: Fix ip segfault when using --color switch
> color: Fix another ip segfault when using --color switch
> color: Cleanup code to remove "magic" offset + 7
> color: Rename enum
>
> include/color.h | 3 +--
> include/json_print.h | 2 +-
> lib/color.c | 17 ++++++++++-------
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
Applied, Thans Petr
^ permalink raw reply
* Re: [iproute2] regression in ss output
From: Stephen Hemminger @ 2017-10-16 16:20 UTC (permalink / raw)
To: Humberto Alves; +Cc: netdev@vger.kernel.org
In-Reply-To: <AM4PR0901MB125261BF480B49D2BA2DF46BCC480@AM4PR0901MB1252.eurprd09.prod.outlook.com>
On Fri, 13 Oct 2017 09:57:37 +0000
Humberto Alves <hjalves@live.com> wrote:
> Hi! With the last iproute2 release, ss command output does not
> differentiate between any-address IPv4 sockets from the IPv6 ones.
> I don't know if this is an expected behaviour, but the old output looks
> more useful. Below I pasted the difference between the old behaviour and
> the new one.
> I am happy to send a patch if you want. Thanks
>
>
> iproute2-4.12
> -------------
>
> $ ss --version
> ss utility, iproute2-ss170705
>
> $ ss -ntl
> State Recv-Q Send-Q Local Address:Port Peer
> Address:Port
> LISTEN 0 100 *:587 *:*
>
> LISTEN 0 100 *:110 *:*
>
> LISTEN 0 100 *:143 *:*
>
> LISTEN 0 128 *:80 *:*
>
> LISTEN 0 128 *:2225 *:*
>
> LISTEN 0 10 127.0.0.1:5010 *:*
>
> LISTEN 0 128 *:20 *:*
>
> LISTEN 0 128 127.0.0.1:20150 *:*
>
> LISTEN 0 128 *:61719 *:*
>
> LISTEN 0 100 *:25 *:*
>
> LISTEN 0 3 *:1723 *:*
>
> LISTEN 0 128 *:17500 *:*
>
> LISTEN 0 128 127.0.0.1:17600 *:*
>
> LISTEN 0 128 *:8000 *:*
>
> LISTEN 0 100 *:993 *:*
>
> LISTEN 0 128 127.0.0.1:17603 *:*
>
> LISTEN 0 100 *:995 *:*
>
> LISTEN 0 100 :::587 :::*
>
> LISTEN 0 100 :::110 :::*
>
> LISTEN 0 100 :::143 :::*
>
> LISTEN 0 128 :::22000 :::*
>
> LISTEN 0 128 :::80 :::*
>
> LISTEN 0 128 :::2225 :::*
>
> LISTEN 0 128 :::20 :::*
>
> LISTEN 0 128 ::1:20150 :::*
>
> LISTEN 0 100 :::25 :::*
>
> LISTEN 0 128 :::17500 :::*
>
> LISTEN 0 128 :::8384 :::*
>
> LISTEN 0 100 :::993 :::*
>
> LISTEN 0 100 :::995 :::*
>
> iproute2-4.13
> -------------
>
> $ ss --version
> ss utility, iproute2-ss170905
>
> $ ss -ntl
> State Recv-Q Send-Q Local Address:Port Peer
> Address:Port
> LISTEN 0 100 *:587 *:*
>
> LISTEN 0 100 *:110 *:*
>
> LISTEN 0 100 *:143 *:*
>
> LISTEN 0 128 *:80 *:*
>
> LISTEN 0 128 *:2225 *:*
>
> LISTEN 0 10 127.0.0.1:5010 *:*
>
> LISTEN 0 128 *:20 *:*
>
> LISTEN 0 128 127.0.0.1:20150 *:*
>
> LISTEN 0 128 *:61719 *:*
>
> LISTEN 0 100 *:25 *:*
>
> LISTEN 0 3 *:1723 *:*
>
> LISTEN 0 128 *:17500 *:*
>
> LISTEN 0 128 127.0.0.1:17600 *:*
>
> LISTEN 0 128 *:8000 *:*
>
> LISTEN 0 100 *:993 *:*
>
> LISTEN 0 128 127.0.0.1:17603 *:*
>
> LISTEN 0 100 *:995 *:*
>
> LISTEN 0 100 *:587 *:*
>
> LISTEN 0 100 *:110 *:*
>
> LISTEN 0 100 *:143 *:*
>
> LISTEN 0 128 *:22000 *:*
>
> LISTEN 0 128 *:80 *:*
>
> LISTEN 0 128 *:2225 *:*
>
> LISTEN 0 128 *:20 *:*
>
> LISTEN 0 128 [::1]:20150 *:*
>
> LISTEN 0 100 *:25 *:*
>
> LISTEN 0 128 *:17500 *:*
>
> LISTEN 0 128 *:8384 *:*
>
> LISTEN 0 100 *:993 *:*
>
> LISTEN 0 100 *:995 *:*
All changes are not regressions.
Yes it was intentional because the outputing [::]:20 looked ugly and using * makes IPv4 and IPv6
look the same.
^ permalink raw reply
* Re: [PATCH net] bpf: disallow arithmetic operations on context pointer
From: Edward Cree @ 2017-10-16 16:16 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <20171016154552.30640-1-jakub.kicinski@netronome.com>
On 16/10/17 16:45, Jakub Kicinski wrote:
> Commit f1174f77b50c ("bpf/verifier: rework value tracking")
> removed the crafty selection of which pointer types are
> allowed to be modified. This is OK for most pointer types
> since adjust_ptr_min_max_vals() will catch operations on
> immutable pointers. One exception is PTR_TO_CTX which is
> now allowed to be offseted freely.
>
> The intent of aforementioned commit was to allow context
> access via modified registers. The offset passed to
> ->is_valid_access() verifier callback has been adjusted
> by the value of the variable offset.
>
> What is missing, however, is taking the variable offset
> into account when the context register is used. Or in terms
> of the code adding the offset to the value passed to the
> ->convert_ctx_access() callback. This leads to the following
Good catch.
So the problem is just that convert_ctx_access() can't deal
with it, yes? Assuming that the offset is constant, because
otherwise we'd reject it anyway, we _could_ stash that offset
in insn_aux_data, and reject any paths that tried to change it
subsequently; and then convert_ctx_access() could be given the
total (off + reg->off) and then the reg->off could be subtracted
from the result, giving the right 'converted' insn offset.
That would then mean that your example,
> eBPF user code:
>
> r1 += 68
> r0 = *(u32 *)(r1 + 8)
> exit
>
> being translated to this in kernel space:
>
> 0: (07) r1 += 68
> 1: (61) r0 = *(u32 *)(r1 +180)
> 2: (95) exit
would instead convert offset 76 to something else, let's say
200 just for the sake of argument, and produce
1: (61) r0 = *(u32 *)(r1 + 132)
which would then do the right thing at run time.
However, I don't know whether anyone would actually want this
to be supported for their programs, and so I'm happy to
disallow this for net and then maybe we can follow up in
net-next with the change I describe above if it's useful.
> Offset 8 is corresponding to 180 in the kernel, but offset
> 76 is valid too. Verifier will "accept" access to offset
> 68+8=76 but then "convert" access to offset 8 as 180.
> Effective access to offset 248 is beyond the kernel context.
> (This is a __sk_buff example on a debug-heavy kernel -
> packet mark is 8 -> 180, 76 would be data.)
>
> Dereferencing the modified context pointer is not as easy
> as dereferencing other types, because we have to translate
> the access to reading a field in kernel structures which is
> usually at a different offset and often of a different size.
> To allow modifying the pointer we would have to make sure
> that given eBPF instruction will always access the same
> field or the fields accessed are "compatible" in terms of
> offset and size...
>
> Disallow dereferencing modified context pointers and add
> to selftests the test case described here.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> Dave, a merge note - in net-next this will need env to be passed
> to verbose().
>
> kernel/bpf/verifier.c | 8 ++++++--
> tools/testing/selftests/bpf/test_verifier.c | 14 ++++++++++++++
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8b8d6ba39e23..8499759d0c7a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1116,7 +1116,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> /* ctx accesses must be at a fixed offset, so that we can
> * determine what type of data were returned.
> */
> - if (!tnum_is_const(reg->var_off)) {
> + if (reg->off) {
> + verbose("derefence of modified ctx ptr R%d off=%d+%d, ctx+const is allowed, ctx+const+const is not\n",
This is slightly unclear, it's not that two adds is bad (e.g. r1 += 8;
r0 = *(u32 *)r1 is bad too), it's that the offset must be in the load,
not the register; your message might be accurate for some compilers but
not in full generality (especially for assemblers without compiling).
Also, sp. "dereference".
> + regno, reg->off, off - reg->off);
> + return -EACCES;
> + }
> + if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
> char tn_buf[48];
>
> tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> @@ -1124,7 +1129,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> tn_buf, off, size);
> return -EACCES;
> }
> - off += reg->var_off.value;
> err = check_ctx_access(env, insn_idx, off, size, t, ®_type);
> if (!err && t == BPF_READ && value_regno >= 0) {
> /* ctx access returns either a scalar, or a
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 26f3250bdcd2..d41c77e7b39b 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -6645,6 +6645,20 @@ static struct bpf_test tests[] = {
> .errstr = "BPF_END uses reserved fields",
> .result = REJECT,
> },
> + {
> + "arithmetic ops make PTR_TO_CTX unusable",
> + .insns = {
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_1,
> + offsetof(struct __sk_buff, data) -
> + offsetof(struct __sk_buff, mark)),
> + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> + offsetof(struct __sk_buff, mark)),
> + BPF_EXIT_INSN(),
> + },
> + .errstr = "derefence of modified ctx ptr R1 off=68+8, ctx+const is allowed, ctx+const+const is not",
> + .result = REJECT,
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + },
> };
>
> static int probe_filter_length(const struct bpf_insn *fp)
^ permalink raw reply
* Re: [PATCH net-next 1/2] tcp: TCP experimental option for SMC
From: Eric Dumazet @ 2017-10-16 16:13 UTC (permalink / raw)
To: Ursula Braun
Cc: davem, netdev, linux-s390, jwi, schwidefsky, heiko.carstens,
raspl
In-Reply-To: <20171016154202.72635-2-ubraun@linux.vnet.ibm.com>
On Mon, 2017-10-16 at 17:42 +0200, Ursula Braun wrote:
...
> +static void smc_set_capability(struct inet_request_sock *ireq,
> + const struct tcp_options_received *rx_opt)
> +{
> +#if IS_ENABLED(CONFIG_SMC)
> + if (!static_key_false(&tcp_have_smc))
> + return;
> +
> + if (rx_opt->smc_ok)
> + ireq->smc_ok = 1;
> + else
> + ireq->smc_ok = 0;
> +#endif
> +}
I do not think this static_key_false(&tcp_have_smc)) makes sense here.
1) If you have a SYN packet while the key is not set
2) Then tcp_have_smc is changed
3) ACK packets comes, and finds random value in ireq->smc_ok
So really here you have to set this field.
static void smc_set_capability(struct inet_request_sock *ireq,
const struct tcp_options_received *rx_opt)
{
#if IS_ENABLED(CONFIG_SMC)
ireq->smc_ok = rx_opt->smc_ok;
#endif
}
^ 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