* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Jes Sorensen @ 2017-10-10 19:35 UTC (permalink / raw)
To: Gustavo A. R. Silva, Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20171010193027.GA23108@embeddedor.com>
On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
While this isn't harmful, to me this looks like pointless patch churn
for zero gain and it's just ugly.
Jes
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 7806a4d..e66be05 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
> switch (hw->conf.chandef.width) {
> case NL80211_CHAN_WIDTH_20_NOHT:
> ht = false;
> + /* fall through */
> case NL80211_CHAN_WIDTH_20:
> opmode |= BW_OPMODE_20MHZ;
> rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
> @@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
> switch (hw->conf.chandef.width) {
> case NL80211_CHAN_WIDTH_20_NOHT:
> ht = false;
> + /* fall through */
> case NL80211_CHAN_WIDTH_20:
> rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
> subchannel = 0;
> @@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv)
> case 3:
> priv->ep_tx_low_queue = 1;
> priv->ep_tx_count++;
> + /* fall through */
> case 2:
> priv->ep_tx_normal_queue = 1;
> priv->ep_tx_count++;
> + /* fall through */
> case 1:
> priv->ep_tx_high_queue = 1;
> priv->ep_tx_count++;
> @@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> break;
> case WLAN_CIPHER_SUITE_TKIP:
> key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> + /* fall through */
> default:
> return -EOPNOTSUPP;
> }
>
^ permalink raw reply
* Re: [Non-DoD Source] Re: [PATCH net-next v2 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: Chenbo Feng @ 2017-10-10 19:42 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, netdev, SELinux,
Daniel Borkmann, Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <1507663408.30616.18.camel@tycho.nsa.gov>
On Tue, Oct 10, 2017 at 12:23 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Tue, 2017-10-10 at 10:48 -0700, Chenbo Feng wrote:
>> On Tue, Oct 10, 2017 at 7:24 AM, Stephen Smalley <sds@tycho.nsa.gov>
>> wrote:
>> > On Mon, 2017-10-09 at 15:20 -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.
>> > > The information stored inside the file security struct is the
>> > > same as
>> > > the information in bpf object security struct.
>> > >
>> > > Signed-off-by: Chenbo Feng <fengc@google.com>
>> > > ---
>> > > include/linux/bpf.h | 3 +++
>> > > include/linux/lsm_hooks.h | 17 +++++++++++++
>> > > include/linux/security.h | 9 +++++++
>> > > kernel/bpf/syscall.c | 4 ++--
>> > > security/security.c | 8 +++++++
>> > > security/selinux/hooks.c | 61
>> > > +++++++++++++++++++++++++++++++++++++++++++++++
>> > > 6 files changed, 100 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/include/linux/lsm_hooks.h
>> > > b/include/linux/lsm_hooks.h
>> > > index 7161d8e7ee79..517dea60b87b 100644
>> > > --- a/include/linux/lsm_hooks.h
>> > > +++ b/include/linux/lsm_hooks.h
>> > > @@ -1385,6 +1385,19 @@
>> > > * @bpf_prog_free_security:
>> > > * Clean up the security information stored inside bpf prog.
>> > > *
>> > > + * @bpf_map_file:
>> > > + * When creating a bpf map fd, set up the file security
>> > > information with
>> > > + * the bpf security information stored in the map struct. So
>> > > when the map
>> > > + * fd is passed between processes, the security module can
>> > > directly read
>> > > + * the security information from file security struct rather
>> > > than the bpf
>> > > + * security struct.
>> > > + *
>> > > + * @bpf_prog_file:
>> > > + * When creating a bpf prog fd, set up the file security
>> > > information with
>> > > + * the bpf security information stored in the prog struct. So
>> > > when the prog
>> > > + * fd is passed between processes, the security module can
>> > > directly read
>> > > + * the security information from file security struct rather
>> > > than the bpf
>> > > + * security struct.
>> > > */
>> > > union security_list_options {
>> > > int (*binder_set_context_mgr)(struct task_struct *mgr);
>> > > @@ -1726,6 +1739,8 @@ union security_list_options {
>> > > void (*bpf_map_free_security)(struct bpf_map *map);
>> > > int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
>> > > void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
>> > > + void (*bpf_map_file)(struct bpf_map *map, struct file
>> > > *file);
>> > > + void (*bpf_prog_file)(struct bpf_prog_aux *aux, struct file
>> > > *file);
>> > > #endif /* CONFIG_BPF_SYSCALL */
>> > > };
>> > >
>> > > @@ -1954,6 +1969,8 @@ struct security_hook_heads {
>> > > struct list_head bpf_map_free_security;
>> > > struct list_head bpf_prog_alloc_security;
>> > > struct list_head bpf_prog_free_security;
>> > > + struct list_head bpf_map_file;
>> > > + struct list_head bpf_prog_file;
>> > > #endif /* CONFIG_BPF_SYSCALL */
>> > > } __randomize_layout;
>> > >
>> > > diff --git a/include/linux/security.h b/include/linux/security.h
>> > > index 18800b0911e5..57573b794e2d 100644
>> > > --- a/include/linux/security.h
>> > > +++ b/include/linux/security.h
>> > > @@ -1740,6 +1740,8 @@ extern int security_bpf_map_alloc(struct
>> > > bpf_map *map);
>> > > extern void security_bpf_map_free(struct bpf_map *map);
>> > > extern int security_bpf_prog_alloc(struct bpf_prog_aux *aux);
>> > > extern void security_bpf_prog_free(struct bpf_prog_aux *aux);
>> > > +extern void security_bpf_map_file(struct bpf_map *map, struct
>> > > file
>> > > *file);
>> > > +extern void security_bpf_prog_file(struct bpf_prog_aux *aux,
>> > > struct
>> > > file *file);
>> > > #else
>> > > static inline int security_bpf(int cmd, union bpf_attr *attr,
>> > > unsigned int size)
>> > > @@ -1772,6 +1774,13 @@ static inline int
>> > > security_bpf_prog_alloc(struct bpf_prog_aux *aux)
>> > >
>> > > static inline void security_bpf_prog_free(struct bpf_prog_aux
>> > > *aux)
>> > > { }
>> > > +
>> > > +static inline void security_bpf_map_file(struct bpf_map *map,
>> > > struct
>> > > file *file)
>> > > +{ }
>> > > +
>> > > +static inline void security_bpf_prog_file(struct bpf_prog_aux
>> > > *aux,
>> > > + struct file *file)
>> > > +{ }
>> > > #endif /* CONFIG_SECURITY */
>> > > #endif /* CONFIG_BPF_SYSCALL */
>> > >
>> > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> > > index 1cf31ddd7616..b144181d3f3a 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
>> > > @@ -964,7 +964,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/security.c b/security/security.c
>> > > index 1cd8526cb0b7..dacf649b8cfa 100644
>> > > --- a/security/security.c
>> > > +++ b/security/security.c
>> > > @@ -1734,4 +1734,12 @@ void security_bpf_prog_free(struct
>> > > bpf_prog_aux *aux)
>> > > {
>> > > call_void_hook(bpf_prog_free_security, aux);
>> > > }
>> > > +void security_bpf_map_file(struct bpf_map *map, struct file
>> > > *file)
>> > > +{
>> > > + call_void_hook(bpf_map_file, map, file);
>> > > +}
>> > > +void security_bpf_prog_file(struct bpf_prog_aux *aux, struct
>> > > file
>> > > *file)
>> > > +{
>> > > + call_void_hook(bpf_prog_file, aux, file);
>> > > +}
>> > > #endif /* CONFIG_BPF_SYSCALL */
>> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> > > index 41aba4e3d57c..fea88655e0ee 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_file_check(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_file_check(file, cred_sid(cred));
>> > > + if (rc)
>> > > + goto out;
>> > > +#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_file_check(file, sid);
>> > > + if (rc)
>> > > + return rc;
>> > > +#endif
>> > > +
>> > > if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>> > > return 0;
>> > >
>> > > @@ -6288,6 +6304,33 @@ 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_file_check(struct file *file, u32 sid)
>> > > +{
>> > > + struct file_security_struct *fsec = file->f_security;
>> > > + int ret;
>> > > +
>> > > + if (file->f_op == &bpf_map_fops) {
>> > > + ret = avc_has_perm(sid, fsec->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) {
>> > > + ret = avc_has_perm(sid, fsec->sid,
>> > > SECCLASS_BPF_PROG,
>> > > + BPF_PROG__USE, NULL);
>> > > + if (ret)
>> > > + return ret;
>> > > + }
>> > > + return 0;
>> > > +}
>> > > +
>> > > static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> > > {
>> > > u32 sid = current_sid();
>> > > @@ -6351,6 +6394,22 @@ static void selinux_bpf_prog_free(struct
>> > > bpf_prog_aux *aux)
>> > > aux->security = NULL;
>> > > kfree(bpfsec);
>> > > }
>> > > +
>> > > +static void selinux_bpf_map_file(struct bpf_map *map, struct
>> > > file
>> > > *file)
>> > > +{
>> > > + struct bpf_security_struct *bpfsec = map->security;
>> > > + struct file_security_struct *fsec = file->f_security;
>> > > +
>> > > + fsec->sid = bpfsec->sid;
>> > > +}
>> > > +
>> > > +static void selinux_bpf_prog_file(struct bpf_prog_aux *aux,
>> > > struct
>> > > file *file)
>> > > +{
>> > > + struct bpf_security_struct *bpfsec = aux->security;
>> > > + struct file_security_struct *fsec = file->f_security;
>> > > +
>> > > + fsec->sid = bpfsec->sid;
>> >
>> > I could be wrong, but isn't it the case that fsec->sid already will
>> > equal bpfsec->sid, because they are both created by the same thread
>> > during the same system call, and they each inherit the SID of the
>> > current task?
>> >
>>
>> This is true when bpf object is created by the same process that
>> obtains the fd. But there are other ways of getting a bpf object fd
>> from the kernel such as bpf_obj_get and bpf_get_obj_fd_by_id. These
>> action will ask the kernel to allocate a new file for the bpf object
>> and the file sid would be the process ask for fd while the bpfsec-
>> >sid
>> is the sid when bpf object get created. These two could be different.
>
> Oh, in that case you shouldn't change the fsec->sid; you'll need to use
> the bpfsec->sid in your checks instead. But you can still do what I
> described below.
>
Okay, I will add a bpf flag and a bpf sid in the file security struct
to store the flag and sid for selinux checking when fd get transfered.
>> > What I expected you to do was to add and set a flags field in the
>> > file_security_struct to indicate that this is a bpf map or prog,
>> > and
>> > then test for that in your bpf_file_check() function instead of
>> > having
>> > to export and test the fops structures.
>> >
>> >
>> > > +}
>> > > #endif
>> > >
>> > > static struct security_hook_list selinux_hooks[]
>> > > __lsm_ro_after_init
>> > > = {
>> > > @@ -6581,6 +6640,8 @@ static struct security_hook_list
>> > > selinux_hooks[] __lsm_ro_after_init = {
>> > > LSM_HOOK_INIT(bpf_prog_alloc_security,
>> > > selinux_bpf_prog_alloc),
>> > > LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
>> > > LSM_HOOK_INIT(bpf_prog_free_security,
>> > > selinux_bpf_prog_free),
>> > > + LSM_HOOK_INIT(bpf_map_file, selinux_bpf_map_file),
>> > > + LSM_HOOK_INIT(bpf_prog_file, selinux_bpf_prog_file),
>> > > #endif
>> > > };
>> > >
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: Add support for new flash parts
From: David Miller @ 2017-10-10 19:51 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, leedom, indranil, venkatesh
In-Reply-To: <1507619653-8726-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Tue, 10 Oct 2017 12:44:13 +0530
> Add support for new flash parts identification, and
> also cleanup the flash Part identifying and decoding
> code.
>
> Based on the original work of Casey Leedom <leedom@chelsio.com>
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: add new T5 pci device id's
From: David Miller @ 2017-10-10 19:52 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1507619702-8793-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Tue, 10 Oct 2017 12:45:02 +0530
> Add 0x50aa and 0x50ab T5 device id's.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs
From: Florian Fainelli @ 2017-10-10 19:55 UTC (permalink / raw)
To: Jes Sorensen, Gustavo A. R. Silva, Kalle Valo
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5f5f0f54-d901-90be-9025-0a1c4b909368-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 10/10/2017 12:35 PM, Jes Sorensen wrote:
> On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
>
> While this isn't harmful, to me this looks like pointless patch churn
> for zero gain and it's just ugly.
That is the canonical way to tell static analyzers and compilers that
fall throughs are wanted and not accidental mistakes in the code. For
people that deal with these kinds of errors, it's quite helpful, unless
you suggest disabling that particular GCC warning specific for that
file/directory?
>
> Jes
>
>
>> Cc: Jes Sorensen <Jes.Sorensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Signed-off-by: Gustavo A. R. Silva <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>> ---
>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 7806a4d..e66be05 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct
>> ieee80211_hw *hw)
>> switch (hw->conf.chandef.width) {
>> case NL80211_CHAN_WIDTH_20_NOHT:
>> ht = false;
>> + /* fall through */
>> case NL80211_CHAN_WIDTH_20:
>> opmode |= BW_OPMODE_20MHZ;
>> rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
>> @@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct
>> ieee80211_hw *hw)
>> switch (hw->conf.chandef.width) {
>> case NL80211_CHAN_WIDTH_20_NOHT:
>> ht = false;
>> + /* fall through */
>> case NL80211_CHAN_WIDTH_20:
>> rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
>> subchannel = 0;
>> @@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct
>> rtl8xxxu_priv *priv)
>> case 3:
>> priv->ep_tx_low_queue = 1;
>> priv->ep_tx_count++;
>> + /* fall through */
>> case 2:
>> priv->ep_tx_normal_queue = 1;
>> priv->ep_tx_count++;
>> + /* fall through */
>> case 1:
>> priv->ep_tx_high_queue = 1;
>> priv->ep_tx_count++;
>> @@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw
>> *hw, enum set_key_cmd cmd,
>> break;
>> case WLAN_CIPHER_SUITE_TKIP:
>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
>> + /* fall through */
>> default:
>> return -EOPNOTSUPP;
>> }
>>
>
--
Florian
^ permalink raw reply
* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Or Gerlitz @ 2017-10-10 20:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
Saeed Mahameed, Matan Barak, Leon Romanovsky, mlxsw
In-Reply-To: <20171010073016.3682-4-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -206,8 +206,6 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
> int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
> int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
> struct net_device **hw_dev);
> -int tcf_exts_egdev_cb_call(struct tcf_exts *exts, enum tc_setup_type type,
> - void *type_data, bool err_stop);
but this (and another 1-2 hunks below) were set by upstream patch of
this series, did you do that add/del on purpose? is that for
bisection? if not why?
^ permalink raw reply
* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Or Gerlitz @ 2017-10-10 20:08 UTC (permalink / raw)
To: Jiri Pirko, Simon Horman
Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
Saeed Mahameed, mlxsw
In-Reply-To: <20171010073016.3682-4-jiri@resnulli.us>
On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> The only user of cls_flower->egress_dev is mlx5.
but nfp supports decap action offload too and from the flower code
stand point, I guess they are both the same, right? how does it work
there?
Or.
^ permalink raw reply
* [PATCH net-next 1/1] veth: tweak creation of veth device
From: Roman Mashak @ 2017-10-10 20:08 UTC (permalink / raw)
To: davem; +Cc: jhs, netdev, Roman Mashak
When creating veth pair, at first rtnl_new_link() creates veth_dev, i.e.
one end of the veth pipe, but not registers it; then veth_newlink() gets
invoked, where peer dev is created _and_ registered, followed by veth_dev
registration, which may fail if peer information, that is VETH_INFO_PEER
attribute, has not been provided and the kernel will allocate unique veth
name.
So, we should ask the kernel to allocate unique name for veth_dev only
when peer info is not available.
Example:
% ip link dev veth0 type veth
RTNETLINK answers: File exists
After fix:
% ip link dev veth0 type veth
% ip link show dev veth0
5: veth0@veth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether f6:ef:8b:96:f4:ec brd ff:ff:ff:ff:ff:ff
%
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
drivers/net/veth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f5438d0..00dce15 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -432,7 +432,7 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
if (tb[IFLA_ADDRESS] == NULL)
eth_hw_addr_random(dev);
- if (tb[IFLA_IFNAME])
+ if (ifmp && tb[IFLA_IFNAME])
nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
else
snprintf(dev->name, IFNAMSIZ, DRV_NAME "%%d");
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 0/5] Support set_ringparam and {set|get}_rxnfc ethtool commands
From: David Miller @ 2017-10-10 20:09 UTC (permalink / raw)
To: lipeng321; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta
In-Reply-To: <1507624927-98008-1-git-send-email-lipeng321@huawei.com>
From: Lipeng <lipeng321@huawei.com>
Date: Tue, 10 Oct 2017 16:42:02 +0800
> 1, Patch [1/5,2/5] add support for ethtool ops set_ringparam
> (ethtool -G) and fix related bug.
> 2, Patch [3/5,4/5, 5/5] add support for ethtool ops
> set_rxnfc/get_rxnfc (-n/-N) and fix related bug.
Series applied, thank you.
^ permalink raw reply
* RE: [PATCH] i40e: mark PM functions as __maybe_unused
From: Keller, Jacob E @ 2017-10-10 20:11 UTC (permalink / raw)
To: Arnd Bergmann, Kirsher, Jeffrey T
Cc: Williams, Mitch A, Duyck, Alexander H, David S. Miller,
Brady, Alan, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20171010081847.4090496-1-arnd@arndb.de>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, October 10, 2017 1:18 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Williams, Mitch A <mitch.a.williams@intel.com>;
> Duyck, Alexander H <alexander.h.duyck@intel.com>; David S. Miller
> <davem@davemloft.net>; Brady, Alan <alan.brady@intel.com>; intel-wired-
> lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] i40e: mark PM functions as __maybe_unused
>
> A cleanup of the PM code left an incorrect #ifdef in place, leading
> to a harmless build warning:
>
> drivers/net/ethernet/intel/i40e/i40e_main.c:12223:12: error: 'i40e_resume'
> defined but not used [-Werror=unused-function]
> drivers/net/ethernet/intel/i40e/i40e_main.c:12185:12: error: 'i40e_suspend'
> defined but not used [-Werror=unused-function]
>
> It's easier to use __maybe_unused attributes here, since you
> can't pick the wrong one.
>
Sure.
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
> Fixes: 0e5d3da40055 ("i40e: use newer generic PM support instead of legacy PM
> callbacks")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 60b11fdeca2d..eb091268bc3c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8370,7 +8370,6 @@ static int i40e_init_interrupt_scheme(struct i40e_pf
> *pf)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> /**
> * i40e_restore_interrupt_scheme - Restore the interrupt scheme
> * @pf: private board data structure
> @@ -8419,7 +8418,6 @@ static int i40e_restore_interrupt_scheme(struct i40e_pf
> *pf)
>
> return err;
> }
> -#endif /* CONFIG_PM */
>
> /**
> * i40e_setup_misc_vector - Setup the misc vector to handle non queue events
> @@ -12177,12 +12175,11 @@ static void i40e_shutdown(struct pci_dev *pdev)
> }
> }
>
> -#ifdef CONFIG_PM
> /**
> * i40e_suspend - PM callback for moving to D3
> * @dev: generic device information structure
> **/
> -static int i40e_suspend(struct device *dev)
> +static int __maybe_unused i40e_suspend(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> struct i40e_pf *pf = pci_get_drvdata(pdev);
> @@ -12220,7 +12217,7 @@ static int i40e_suspend(struct device *dev)
> * i40e_resume - PM callback for waking up from D3
> * @dev: generic device information structure
> **/
> -static int i40e_resume(struct device *dev)
> +static int __maybe_unused i40e_resume(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> struct i40e_pf *pf = pci_get_drvdata(pdev);
> @@ -12252,8 +12249,6 @@ static int i40e_resume(struct device *dev)
> return 0;
> }
>
> -#endif /* CONFIG_PM */
> -
> static const struct pci_error_handlers i40e_err_handler = {
> .error_detected = i40e_pci_error_detected,
> .slot_reset = i40e_pci_error_slot_reset,
> @@ -12269,11 +12264,9 @@ static struct pci_driver i40e_driver = {
> .id_table = i40e_pci_tbl,
> .probe = i40e_probe,
> .remove = i40e_remove,
> -#ifdef CONFIG_PM
> .driver = {
> .pm = &i40e_pm_ops,
> },
> -#endif /* CONFIG_PM */
> .shutdown = i40e_shutdown,
> .err_handler = &i40e_err_handler,
> .sriov_configure = i40e_pci_sriov_configure,
> --
> 2.9.0
^ permalink raw reply
* Re: [PATCH net-next 0/3] mlx4_en num of rings
From: David Miller @ 2017-10-10 20:11 UTC (permalink / raw)
To: tariqt; +Cc: netdev, eranbe
In-Reply-To: <1507627715-25487-1-git-send-email-tariqt@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 10 Oct 2017 12:28:32 +0300
> This patchset from Inbar contains changes to rings control
> to the mlx4 Eth driver.
>
> Patches 1 and 2 limit the number of rings to the number of CPUs.
> Patch 3 removes a limitation in logic of default number of RX rings.
>
> Series generated against net-next commit:
> 812b5ca7d376 Add a driver for Renesas uPD60620 and uPD60620A PHYs
Series applied, thanks Tariq.
^ permalink raw reply
* Re: [PATCH net-next] selftests: rtnetlink: test RTM_GETNETCONF
From: David Miller @ 2017-10-10 20:15 UTC (permalink / raw)
To: fw; +Cc: netdev
In-Reply-To: <20171010141805.19194-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Tue, 10 Oct 2017 16:18:05 +0200
> exercise RTM_GETNETCONF call path for unspec, inet and inet6
> families, they are DOIT_UNLOCKED candidates.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied, thanks Florian.
^ permalink raw reply
* Re: [PATCH v4 net-next] rtnetlink: bridge: use ext_ack instead of printk
From: David Miller @ 2017-10-10 20:16 UTC (permalink / raw)
To: fw; +Cc: netdev, dsahern
In-Reply-To: <20171010151004.20056-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Tue, 10 Oct 2017 17:10:04 +0200
> We can now piggyback error strings to userspace via extended acks
> rather than using printk.
>
> Before:
> bridge fdb add 01:02:03:04:05:06 dev br0 vlan 4095
> RTNETLINK answers: Invalid argument
>
> After:
> bridge fdb add 01:02:03:04:05:06 dev br0 vlan 4095
> Error: invalid vlan id.
>
> v3: drop 'RTM_' prefixes, suggested by David Ahern, they
> are not useful, the add/del in bridge command line is enough.
>
> Also reword error in response to malformed/bad vlan id attribute
> size.
>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied, thanks.
^ permalink raw reply
* Re: [net 0/2][pull request] Intel Wired LAN Driver Updates 2017-10-10
From: David Miller @ 2017-10-10 20:17 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20171010151416.43149-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Oct 2017 08:14:14 -0700
> This series contains updates to i40e only.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net 0/2] nfp: fix ethtool stats and page allocation
From: David Miller @ 2017-10-10 20:18 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20171010161623.23838-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 10 Oct 2017 09:16:21 -0700
> Two fixes for net. First one makes sure we handle gather of stats on
> 32bit machines correctly (ouch). The second fix solves a potential
> NULL-deref if we fail to allocate a page with XDP running.
>
> I used Fixes: tags pointing to where the bug was introduced, but for
> patch 1 it has been in the driver "for ever" and fix won't backport
> cleanly beyond commit 325945ede6d4 ("nfp: split software and hardware
> vNIC statistics") which is in net.
Series applied, thanks Jakub.
^ permalink raw reply
* Re: [net-next 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2017-10-10
From: David Miller @ 2017-10-10 20:21 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20171010172139.77914-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 10 Oct 2017 10:21:30 -0700
> This series contains updates to e1000e and igb.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [ovs-dev] [PATCH net-next] openvswitch: add ct_clear action
From: Joe Stringer @ 2017-10-10 20:24 UTC (permalink / raw)
To: Eric Garver, Joe Stringer, Pravin Shelar,
Linux Kernel Network Developers, ovs dev
In-Reply-To: <20171010191329.GH26353@dev-rhel7>
On 10 October 2017 at 12:13, Eric Garver <e@erig.me> wrote:
> On Tue, Oct 10, 2017 at 10:24:20AM -0700, Joe Stringer wrote:
>> On 10 October 2017 at 08:09, Eric Garver <e@erig.me> wrote:
>> > On Tue, Oct 10, 2017 at 05:33:48AM -0700, Joe Stringer wrote:
>> >> On 9 October 2017 at 21:41, Pravin Shelar <pshelar@ovn.org> wrote:
>> >> > On Fri, Oct 6, 2017 at 9:44 AM, Eric Garver <e@erig.me> wrote:
>> >> >> This adds a ct_clear action for clearing conntrack state. ct_clear is
>> >> >> currently implemented in OVS userspace, but is not backed by an action
>> >> >> in the kernel datapath. This is useful for flows that may modify a
>> >> >> packet tuple after a ct lookup has already occurred.
>> >> >>
>> >> >> Signed-off-by: Eric Garver <e@erig.me>
>> >> > Patch mostly looks good. I have following comments.
>> >> >
>> >> >> ---
>> >> >> include/uapi/linux/openvswitch.h | 2 ++
>> >> >> net/openvswitch/actions.c | 5 +++++
>> >> >> net/openvswitch/conntrack.c | 12 ++++++++++++
>> >> >> net/openvswitch/conntrack.h | 7 +++++++
>> >> >> net/openvswitch/flow_netlink.c | 5 +++++
>> >> >> 5 files changed, 31 insertions(+)
>> >> >>
>> >> >> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> >> >> index 156ee4cab82e..1b6e510e2cc6 100644
>> >> >> --- a/include/uapi/linux/openvswitch.h
>> >> >> +++ b/include/uapi/linux/openvswitch.h
>> >> >> @@ -806,6 +806,7 @@ struct ovs_action_push_eth {
>> >> >> * packet.
>> >> >> * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
>> >> >> * packet.
>> >> >> + * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
>> >> >> *
>> >> >> * Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
>> >> >> * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
>> >> >> @@ -835,6 +836,7 @@ enum ovs_action_attr {
>> >> >> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
>> >> >> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
>> >> >> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
>> >> >> + OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
>> >> >>
>> >> >> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
>> >> >> * from userspace. */
>> >> >> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> >> >> index a54a556fcdb5..db9c7f2e662b 100644
>> >> >> --- a/net/openvswitch/actions.c
>> >> >> +++ b/net/openvswitch/actions.c
>> >> >> @@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> >> return err == -EINPROGRESS ? 0 : err;
>> >> >> break;
>> >> >>
>> >> >> + case OVS_ACTION_ATTR_CT_CLEAR:
>> >> >> + err = ovs_ct_clear(skb, key);
>> >> >> + break;
>> >> >> +
>> >> >> case OVS_ACTION_ATTR_PUSH_ETH:
>> >> >> err = push_eth(skb, key, nla_data(a));
>> >> >> break;
>> >> >> @@ -1210,6 +1214,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>> >> >> case OVS_ACTION_ATTR_POP_ETH:
>> >> >> err = pop_eth(skb, key);
>> >> >> break;
>> >> >> +
>> >> >> }
>> >> > Unrelated change.
>> >> >
>> >> >>
>> >> >> if (unlikely(err)) {
>> >> >> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> >> >> index d558e882ca0c..f9b73c726ad7 100644
>> >> >> --- a/net/openvswitch/conntrack.c
>> >> >> +++ b/net/openvswitch/conntrack.c
>> >> >> @@ -1129,6 +1129,18 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
>> >> >> return err;
>> >> >> }
>> >> >>
>> >> >> +int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
>> >> >> +{
>> >> >> + if (skb_nfct(skb)) {
>> >> >> + nf_conntrack_put(skb_nfct(skb));
>> >> >> + nf_ct_set(skb, NULL, 0);
>> >> > Can the new conntract state be appropriate? may be IP_CT_UNTRACKED?
>> >> >
>> >> >> + }
>> >> >> +
>> >> >> + ovs_ct_fill_key(skb, key);
>> >> >> +
>> >> > I do not see need to refill the key if there is no skb-nf-ct.
>> >>
>> >> Really this is trying to just zero the CT key fields, but reuses
>> >> existing functions, right? This means that subsequent upcalls, for
>> >
>> > Right.
>> >
>> >> instance, won't have the outdated view of the CT state from the
>> >> previous lookup (that was prior to the ct_clear). I'd expect these key
>> >> fields to be cleared.
>> >
>> > I assumed Pravin was saying that we don't need to clear them if there is
>> > no conntrack state. They should already be zero.
>>
>> The conntrack calls aren't going to clear it, so I don't see what else
>> would clear it?
>>
>> If you execute ct(),ct_clear(), then the first ct will set the
>> values.. what will zero them?
>
> I meant move ovs_ct_fill_key() to inside the if statement.
> i.e.
>
> if (skb_nfct(skb)) {
> nf_conntrack_put(skb_nfct(skb));
> nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
> ovs_ct_fill_key(skb, key);
> }
>
> Should be nothing to fill/zero if we have not yet done conntrack.
> Is there a case where we may lose skb->_nfct, but the key still has
> conntrack data?
Ah, misreading on my part. Right, if there is no nfct then it should
already have the right value. I don't think there's a way to lose it
and leave the key with conntrack data.
^ permalink raw reply
* [PATCH net-next v2] openvswitch: add ct_clear action
From: Eric Garver @ 2017-10-10 20:54 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA; +Cc: dev-yBygre7rU0TnMu66kgdUjQ
This adds a ct_clear action for clearing conntrack state. ct_clear is
currently implemented in OVS userspace, but is not backed by an action
in the kernel datapath. This is useful for flows that may modify a
packet tuple after a ct lookup has already occurred.
Signed-off-by: Eric Garver <e@erig.me>
---
v2:
- Use IP_CT_UNTRACKED for nf_ct_set()
- Only fill key if previously conntracked
include/uapi/linux/openvswitch.h | 2 ++
net/openvswitch/actions.c | 4 ++++
net/openvswitch/conntrack.c | 11 +++++++++++
net/openvswitch/conntrack.h | 7 +++++++
net/openvswitch/flow_netlink.c | 5 +++++
5 files changed, 29 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index efdbfbfd3ee2..0cd6f8833147 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -807,6 +807,7 @@ struct ovs_action_push_eth {
* packet.
* @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
* packet.
+ * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
*
* Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
* fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -836,6 +837,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
OVS_ACTION_ATTR_POP_ETH, /* No argument. */
+ OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
__OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
* from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index a54a556fcdb5..a551232daf61 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1203,6 +1203,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
return err == -EINPROGRESS ? 0 : err;
break;
+ case OVS_ACTION_ATTR_CT_CLEAR:
+ err = ovs_ct_clear(skb, key);
+ break;
+
case OVS_ACTION_ATTR_PUSH_ETH:
err = push_eth(skb, key, nla_data(a));
break;
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index d558e882ca0c..fe861e2f0deb 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1129,6 +1129,17 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
return err;
}
+int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ if (skb_nfct(skb)) {
+ nf_conntrack_put(skb_nfct(skb));
+ nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+ ovs_ct_fill_key(skb, key);
+ }
+
+ return 0;
+}
+
static int ovs_ct_add_helper(struct ovs_conntrack_info *info, const char *name,
const struct sw_flow_key *key, bool log)
{
diff --git a/net/openvswitch/conntrack.h b/net/openvswitch/conntrack.h
index bc7efd1867ab..399dfdd2c4f9 100644
--- a/net/openvswitch/conntrack.h
+++ b/net/openvswitch/conntrack.h
@@ -30,6 +30,7 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *, struct sk_buff *);
int ovs_ct_execute(struct net *, struct sk_buff *, struct sw_flow_key *,
const struct ovs_conntrack_info *);
+int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key);
void ovs_ct_fill_key(const struct sk_buff *skb, struct sw_flow_key *key);
int ovs_ct_put_key(const struct sw_flow_key *swkey,
@@ -73,6 +74,12 @@ static inline int ovs_ct_execute(struct net *net, struct sk_buff *skb,
return -ENOTSUPP;
}
+static inline int ovs_ct_clear(struct sk_buff *skb,
+ struct sw_flow_key *key)
+{
+ return -ENOTSUPP;
+}
+
static inline void ovs_ct_fill_key(const struct sk_buff *skb,
struct sw_flow_key *key)
{
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fc0ca9a89b8e..dc0d79092e74 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -76,6 +76,7 @@ static bool actions_may_change_flow(const struct nlattr *actions)
break;
case OVS_ACTION_ATTR_CT:
+ case OVS_ACTION_ATTR_CT_CLEAR:
case OVS_ACTION_ATTR_HASH:
case OVS_ACTION_ATTR_POP_ETH:
case OVS_ACTION_ATTR_POP_MPLS:
@@ -2528,6 +2529,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash),
[OVS_ACTION_ATTR_CT] = (u32)-1,
+ [OVS_ACTION_ATTR_CT_CLEAR] = 0,
[OVS_ACTION_ATTR_TRUNC] = sizeof(struct ovs_action_trunc),
[OVS_ACTION_ATTR_PUSH_ETH] = sizeof(struct ovs_action_push_eth),
[OVS_ACTION_ATTR_POP_ETH] = 0,
@@ -2669,6 +2671,9 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
skip_copy = true;
break;
+ case OVS_ACTION_ATTR_CT_CLEAR:
+ break;
+
case OVS_ACTION_ATTR_PUSH_ETH:
/* Disallow pushing an Ethernet header if one
* is already present */
--
2.12.0
^ permalink raw reply related
* Re: [patch net-next 0/4] net: sched: get rid of cls_flower->egress_dev
From: Jiri Pirko @ 2017-10-10 21:13 UTC (permalink / raw)
To: Or Gerlitz
Cc: Saeed Mahameed, Linux Netdev List, David Miller, Jamal Hadi Salim,
Cong Wang, mlxsw
In-Reply-To: <CAJ3xEMgPdcVrogHZmEnH+vP3E53zvEcFN8+wyuEqSs6utHQVRg@mail.gmail.com>
Tue, Oct 10, 2017 at 07:24:21PM CEST, gerlitz.or@gmail.com wrote:
>Jiri,
>
>FWIW, as I reported to you earlier, I was playing with tc encap/decap rules
>on 4.14-rc+ (net) before
>applying any patch of this series, and something is messy w.r.t to decap
>rules. I don't see
>them removed at all when user space attempts to do so. It might (probably)
>mlx5 bug, which
>we will have to fix for net and later rebase net-next over net. We have
>short WW here so
>I will not be able to do RCA this week.
Or, as I replied to you earlier, the issue you describe is totally
unrelated to this patchset as you see the issue with the current net-next.
Not sure why you add this comment here.
^ permalink raw reply
* Re: [patch net-next 3/4] net: sched: convert cls_flower->egress_dev users to tc_setup_cb_egdev infra
From: Jiri Pirko @ 2017-10-10 21:16 UTC (permalink / raw)
To: Or Gerlitz
Cc: Simon Horman, Linux Netdev List, David Miller, Jamal Hadi Salim,
Cong Wang, Saeed Mahameed, mlxsw
In-Reply-To: <CAJ3xEMh0cM3FszEjKNNe3aWTjHoDxof9GURgUSMy5k3iSTLg2g@mail.gmail.com>
Tue, Oct 10, 2017 at 10:08:23PM CEST, gerlitz.or@gmail.com wrote:
>On Tue, Oct 10, 2017 at 10:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> The only user of cls_flower->egress_dev is mlx5.
>
>but nfp supports decap action offload too and from the flower code
>stand point, I guess they are both the same, right? how does it work
>there?
Apparently they don't use cls_flower->egress_dev.
^ permalink raw reply
* Re: [patch net-next 1/4] net: sched: make tc_action_ops->get_dev return dev and avoid passing net
From: Jiri Pirko @ 2017-10-10 21:19 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Saeed Mahameed, matanb, leonro, mlxsw
In-Reply-To: <CAM_iQpXjq1QVVAC2L3TdTEJzdda1=H1V=c4n5v+qeYJqCSxoWw@mail.gmail.com>
Tue, Oct 10, 2017 at 07:44:53PM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Oct 10, 2017 at 12:30 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> -static int tcf_mirred_device(const struct tc_action *a, struct net *net,
>> - struct net_device **mirred_dev)
>> +static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
>> {
>> - int ifindex = tcf_mirred_ifindex(a);
>> + struct tcf_mirred *m = to_mirred(a);
>>
>> - *mirred_dev = __dev_get_by_index(net, ifindex);
>> - if (!*mirred_dev)
>> - return -EINVAL;
>> - return 0;
>> + return __dev_get_by_index(m->net, m->tcfm_ifindex);
>
>Hmm, why not just return m->tcfm_dev?
I just follow the existing code. The change you suggest should be a
separate follow-up patch.
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: kbuild test robot @ 2017-10-10 21:30 UTC (permalink / raw)
To: Chenbo Feng
Cc: kbuild-all, linux-security-module, netdev, SELinux,
Jeffrey Vander Stoep, Alexei Starovoitov, lorenzo,
Daniel Borkmann, Stephen Smalley, Chenbo Feng
In-Reply-To: <20171009222028.13096-5-chenbofeng.kernel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
Hi Chenbo,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Chenbo-Feng/bpf-security-New-file-mode-and-LSM-hooks-for-eBPF-object-permission-control/20171011-010349
config: x86_64-randconfig-u0-10110310 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/init.h:4:0,
from security/selinux/hooks.c:27:
security/selinux/hooks.c: In function 'bpf_map_fmode_to_av':
security/selinux/hooks.c:6284:6: error: 'f_mode' undeclared (first use in this function)
if (f_mode & FMODE_READ)
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> security/selinux/hooks.c:6284:2: note: in expansion of macro 'if'
if (f_mode & FMODE_READ)
^~
security/selinux/hooks.c:6284:6: note: each undeclared identifier is reported only once for each function it appears in
if (f_mode & FMODE_READ)
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> security/selinux/hooks.c:6284:2: note: in expansion of macro 'if'
if (f_mode & FMODE_READ)
^~
vim +/if +6284 security/selinux/hooks.c
6279
6280 static u32 bpf_map_fmode_to_av(fmode_t fmode)
6281 {
6282 u32 av = 0;
6283
> 6284 if (f_mode & FMODE_READ)
6285 av |= BPF_MAP__READ;
6286 if (f_mode & FMODE_WRITE)
6287 av |= BPF_MAP__WRITE;
6288 return av;
6289 }
6290
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31549 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 6/7] bpf: don't rely on the verifier lock for metadata_dst allocation
From: kbuild test robot @ 2017-10-10 21:33 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kbuild-all, netdev, oss-drivers, alexei.starovoitov, daniel,
Jakub Kicinski
In-Reply-To: <20171009173015.23520-7-jakub.kicinski@netronome.com>
[-- Attachment #1: Type: text/plain, Size: 866 bytes --]
Hi Jakub,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jakub-Kicinski/bpf-get-rid-of-global-verifier-state-and-reuse-instruction-printer/20171011-021905
config: x86_64-randconfig-a0-10110234 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
net//core/dst.c: In function 'metadata_dst_free_percpu':
>> net//core/dst.c:328: warning: unused variable 'cpu'
vim +/cpu +328 net//core/dst.c
325
326 void metadata_dst_free_percpu(struct metadata_dst __percpu *md_dst)
327 {
> 328 int cpu;
329
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30274 bytes --]
^ permalink raw reply
* Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()
From: Cong Wang @ 2017-10-10 21:37 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linux Kernel Network Developers, Eric Dumazet, Yuchung Cheng,
Neal Cardwell, Martin KaFai Lau, Brendan Gregg,
Hannes Frederic Sowa, Song Liu
In-Reply-To: <20171010173821.6djxyvrggvaivqec@ast-mbp.dhcp.thefacebook.com>
On Tue, Oct 10, 2017 at 10:38 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> I'm happy to see new tracepoints being added to tcp stack, but I'm concerned
> with practical usability of them.
> Like the above tracepoint definition makes it not very useful from bpf point of view,
> since 'sk' pointer is not recored by as part of the tracepoint.
> In bpf/tracing world we prefer tracepoints to have raw pointers recorded
> in TP_STRUCT__entry() and _not_ printed in TP_printk()
> (since pointers are useless for userspace).
> Like trace_kfree_skb() tracepoint records raw 'skb' pointer and we can
> walk whatever sk_buff fields we need inside the program.
> Such approach allows tracepoint to be usable in many more scenarios, since
> bpf program can examine kernel datastructures.
Sure, I am happy to add them for BPF. The current version is merely
for our own use case, other use cases like this are always welcome!
> Over the last few years we've been running tcp statistics framework (similar to web10g)
> using 8 kprobes in tcp stack with bpf programs extracting the data and now we're
> ready to share this experience with the community. Right now we're working on a set
> of tracepoints for tcp stack to make the interface more accurate, faster and more stable.
> We're planning to send an RFC patch with these new tracepoints in the comming weeks.
Great! Looking forward to it!
>
> More concrete, if you can make this trace_tcp_retransmit_skb() to record
> sk, skb pointers and err code at the end of __tcp_retransmit_skb() it will solve
> our need as well.
Note, currently I only call trace_tcp_retransmit_skb() for successful
retransmissions, since you mentioned err code, I guess you want it
for failures too? I am not sure if tracing unsuccessful TCP retransmissions
is meaningful here, I guess it's needed for BPF to track TCP states?
It doesn't harm to add it, at least we can filter out err!=0 since we
only care about successful ones.
>
> So far our list of kprobes is:
> int kprobe__tcp_validate_incoming
> int kprobe__tcp_send_active_reset
> int kprobe__tcp_v4_send_reset
> int kprobe__tcp_v6_send_reset
> int kprobe__tcp_v4_destroy_sock
> int kprobe__tcp_set_state
> int kprobe__tcp_retransmit_skb
> int kprobe__tcp_rtx_synack
>
> with tracepoints we can consolidate two of them into one and drop
> another one for sure. Notice that tcp_retransmit_skb is on our list too
> and currently we're doing extra work inside the program to make it more
> accurate which will be unnecessary if this tracepoint is at the end
> of __tcp_retransmit_skb().
Yeah, with these tracepoints we would be able to trace more TCP
state changes.
Thanks!
^ permalink raw reply
* Re: [PATCH v2] xdp: Sample xdp program implementing ip forward
From: David Daney @ 2017-10-10 21:37 UTC (permalink / raw)
To: Stephen Hemminger, Christina Jacob
Cc: Christina Jacob, daniel, Sunil.Goutham, netdev, dsahern,
linux-kernel, brouer, linux-arm-kernel
In-Reply-To: <20171010101921.30136d48@shemminger-XPS-13-9360>
On 10/10/2017 10:19 AM, Stephen Hemminger wrote:
> On Tue, 10 Oct 2017 12:58:52 +0530
> Christina Jacob <christina.jacob.koikara@gmail.com> wrote:
>
>> +/* Get the mac address of the interface given interface name */
>> +static long *getmac(char *iface)
>> +{
>> + int fd;
>> + struct ifreq ifr;
>> + long *mac = NULL;
>> +
>> + fd = socket(AF_INET, SOCK_DGRAM, 0);
>> + ifr.ifr_addr.sa_family = AF_INET;
>> + strncpy(ifr.ifr_name, iface, IFNAMSIZ - 1);
>> + ioctl(fd, SIOCGIFHWADDR, &ifr);
>> + mac = (long *)ifr.ifr_hwaddr.sa_data;
>> + close(fd);
>> + return mac;
>
> Always check return value of ioctl.
> You are assuming sizeof(long) > 6 bytes.
> Also the byte order.
Also:
Returning the address of a local variable (ifr.ifr_hwaddr.sa_data), and
then dereferencing it outside of the function is not correct.
The casting of the char sa_data[] to a long * may cause alignment faults
on some architectures. The may also be endinaness issues depending on
how the data are manipulated if you pack all those chars into a long.
If we think that a MAC address is char[6], then it may be best to define
the data structures as such and manipulate it as an array instead of
trying to pack it into a long.
Keep working on this though, this program will surely be useful.
David Daney
^ 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