* [PATCH V2] backport: handle change in netdevice destructor usage
@ 2017-06-21 20:58 Arend van Spriel
2017-06-21 21:17 ` Johannes Berg
2017-06-21 21:18 ` Johannes Berg
0 siblings, 2 replies; 7+ messages in thread
From: Arend van Spriel @ 2017-06-21 20:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: backports, Arend van Spriel
This patch deals with changes made in struct net_device by commit
cf124db566e6 ("net: Fix inconsistent teardown and release of private
netdev state.").
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Hi Johannes,
It took a while for me to be happy with this version. More so because I
found that brcmfmac has a double-free bug if register_netdevice() fails
so I needed to solve that puzzle. Below is the changelog.
Regards,
Arend
V2:
- changed netdev_set_priv_destructor() to macro.
- revised semantic patch covering drivers only needing free_netdev().
- used some tricks suggested by Julia in semantic patch.
- added more version checks against 4.13.
---
backport/backport-include/linux/netdevice.h | 9 ++++
patches/0079-netdev-destructor.cocci | 84 +++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
create mode 100644 patches/0079-netdev-destructor.cocci
diff --git a/backport/backport-include/linux/netdevice.h b/backport/backport-include/linux/netdevice.h
index 06230b5..0f5b198 100644
--- a/backport/backport-include/linux/netdevice.h
+++ b/backport/backport-include/linux/netdevice.h
@@ -320,4 +320,13 @@ static inline void netif_trans_update(struct net_device *dev)
}
#endif
+#if LINUX_VERSION_IS_LESS(4,13,0)
+#define netdev_set_priv_destructor(_dev, _destructor) \
+ (_dev)->destructor = __ ## _destructor
+#else
+ (_dev)->needs_free_netdev = true;
+ if (_destructor != free_netdev)
+ (_dev)->priv_destructor = _destructor;
+#endif
+
#endif /* __BACKPORT_NETDEVICE_H */
diff --git a/patches/0079-netdev-destructor.cocci b/patches/0079-netdev-destructor.cocci
new file mode 100644
index 0000000..fab8af1
--- /dev/null
+++ b/patches/0079-netdev-destructor.cocci
@@ -0,0 +1,84 @@
+@r1@
+struct net_device *NDEV;
+identifier D, C;
+identifier TRUE =~ "true";
+@@
+C(...)
+{
+ <...
+- NDEV->needs_free_netdev = TRUE;
+- NDEV->priv_destructor = D;
++ netdev_set_priv_destructor(NDEV, D);
+ ...>
+}
+
+@r2 depends on r1@
+identifier r1.D, r1.C;
+fresh identifier E = "__" ## D;
+@@
+
++#if LINUX_VERSION_IS_LESS(4,13,0)
++static void E(struct net_device *ndev)
++{
++ D(ndev);
++ free_netdev(ndev);
++}
++#endif
++
+C(...)
+{
+ ...
+}
+
+@r3 depends on r1@
+type T;
+identifier NDEV;
+identifier r1.D;
+T RET;
+@@
+
+RET = \(register_netdevice\|register_ndev\)(NDEV);
+if (<+... RET ...+>) {
+ <...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++ D(NDEV);
++#endif
+ free_netdev(NDEV);
+ ...>
+}
+
+@r4 depends on r1@
+identifier NDEV;
+identifier r1.D;
+type T;
+T RET;
+@@
+
+if (...)
+ RET = register_netdevice(NDEV);
+else
+ RET = register_netdev(NDEV);
+if (<+... RET ...+>) {
+ <...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++ D(NDEV);
++#endif
+ free_netdev(NDEV);
+ ...>
+}
+
+@r5@
+struct net_device *NDEV;
+identifier TRUE =~ "true";
+@@
+
+-NDEV->needs_free_netdev = TRUE;
++netdev_set_priv_destructor(NDEV, free_netdev);
+
+@r6@
+struct net_device *NDEV;
+identifier D;
+@@
+
+-NDEV->priv_destructor = D;
++netdev_set_priv_destructor(NDEV, D);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2] backport: handle change in netdevice destructor usage
2017-06-21 20:58 [PATCH V2] backport: handle change in netdevice destructor usage Arend van Spriel
@ 2017-06-21 21:17 ` Johannes Berg
2017-06-21 21:18 ` Johannes Berg
1 sibling, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2017-06-21 21:17 UTC (permalink / raw)
To: Arend van Spriel; +Cc: backports
On Wed, 2017-06-21 at 21:58 +0100, Arend van Spriel wrote:
> This patch deals with changes made in struct net_device by commit
> cf124db566e6 ("net: Fix inconsistent teardown and release of private
> netdev state.").
>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
> Hi Johannes,
>
> It took a while for me to be happy with this version. More so because
> I
> found that brcmfmac has a double-free bug if register_netdevice()
> fails
> so I needed to solve that puzzle. Below is the changelog.
>
> Regards,
> Arend
>
> V2:
> - changed netdev_set_priv_destructor() to macro.
> - revised semantic patch covering drivers only needing
> free_netdev().
> - used some tricks suggested by Julia in semantic patch.
> - added more version checks against 4.13.
> ---
> backport/backport-include/linux/netdevice.h | 9 ++++
> patches/0079-netdev-destructor.cocci | 84
> +++++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
> create mode 100644 patches/0079-netdev-destructor.cocci
>
> diff --git a/backport/backport-include/linux/netdevice.h
> b/backport/backport-include/linux/netdevice.h
> index 06230b5..0f5b198 100644
> --- a/backport/backport-include/linux/netdevice.h
> +++ b/backport/backport-include/linux/netdevice.h
> @@ -320,4 +320,13 @@ static inline void netif_trans_update(struct
> net_device *dev)
> }
> #endif
>
> +#if LINUX_VERSION_IS_LESS(4,13,0)
> +#define netdev_set_priv_destructor(_dev, _destructor) \
> + (_dev)->destructor = __ ## _destructor
> +#else
Won't this cause annoying warnings, that __##_destructor is an unused
function, when compiling for higher kernels?
johannes
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] backport: handle change in netdevice destructor usage
2017-06-21 20:58 [PATCH V2] backport: handle change in netdevice destructor usage Arend van Spriel
2017-06-21 21:17 ` Johannes Berg
@ 2017-06-21 21:18 ` Johannes Berg
2017-06-21 21:50 ` Arend van Spriel
1 sibling, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2017-06-21 21:18 UTC (permalink / raw)
To: Arend van Spriel; +Cc: backports
Perhaps that can be fixed as such:
> +@r2 depends on r1@
> +identifier r1.D, r1.C;
> +fresh identifier E = "__" ## D;
attribute __maybe_unused;
> +@@
> +
> ++#if LINUX_VERSION_IS_LESS(4,13,0)
> ++static void E(struct net_device *ndev)
__maybe_unused
johannes
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] backport: handle change in netdevice destructor usage
2017-06-21 21:18 ` Johannes Berg
@ 2017-06-21 21:50 ` Arend van Spriel
2017-06-21 21:52 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Arend van Spriel @ 2017-06-21 21:50 UTC (permalink / raw)
To: Johannes Berg; +Cc: backports
On 21-06-17 23:18, Johannes Berg wrote:
> Perhaps that can be fixed as such:
>
>> +@r2 depends on r1@
>> +identifier r1.D, r1.C;
>> +fresh identifier E = "__" ## D;
>
> attribute __maybe_unused;
>
>> +@@
>> +
>> ++#if LINUX_VERSION_IS_LESS(4,13,0)
>> ++static void E(struct net_device *ndev)
>
> __maybe_unused
Ok. I clearly made a mistake in #else of the macro:
+#if LINUX_VERSION_IS_LESS(4,13,0)
+#define netdev_set_priv_destructor(_dev, _destructor) \
+ (_dev)->destructor = __ ## _destructor
+#else
+ (_dev)->needs_free_netdev = true;
+ if (_destructor != free_netdev)
+ (_dev)->priv_destructor = _destructor;
+#endif
So I will fix that, but I do not quite understand your comment.
Expanding the macro in spatch output for net/mac80211/iface.c:
+#if LINUX_VERSION_IS_LESS(4,13,0)
+static void __ieee80211_if_free(struct net_device *ndev)
+{
+ ieee80211_if_free(ndev);
+ free_netdev(ndev);
+}
+#endif
+
static void ieee80211_if_setup(struct net_device *dev)
{
ether_setup(dev);
dev->priv_flags &= ~IFF_TX_SKB_SHARING;
dev->netdev_ops = &ieee80211_dataif_ops;
- dev->needs_free_netdev = true;
- dev->priv_destructor = ieee80211_if_free;
+#if LINUX_VERSION_IS_LESS(4,13,0)
+ dev->destructor = __ieee80211_if_free
+#else
+ dev->needs_free_netdev = true;
+ if (ieee80211_if_free != free_netdev)
+ dev->priv_destructor = ieee80211_if_free;
+#endif
}
So __ieee80211_if_free is only defined for lower kernels and it is only
used for lower kernels. Adding the __ieee80211_if_free depends on r1 in
the semantic patch.
So I will send a V3 fixing the macro in netdevice.h, but I don't think
we need the maybe_unused attribute.
Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] backport: handle change in netdevice destructor usage
2017-06-21 21:50 ` Arend van Spriel
@ 2017-06-21 21:52 ` Johannes Berg
2017-06-21 21:53 ` Arend van Spriel
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2017-06-21 21:52 UTC (permalink / raw)
To: Arend van Spriel; +Cc: backports
On Wed, 2017-06-21 at 23:50 +0200, Arend van Spriel wrote:
>
> > > ++#if LINUX_VERSION_IS_LESS(4,13,0)
> > > ++static void E(struct net_device *ndev)
> So I will send a V3 fixing the macro in netdevice.h, but I don't
> think we need the maybe_unused attribute.
Ummm, yeah, I totally missed the #if above :)
Maybe it's time to get some sleep...
johannes
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] backport: handle change in netdevice destructor usage
2017-06-21 21:52 ` Johannes Berg
@ 2017-06-21 21:53 ` Arend van Spriel
0 siblings, 0 replies; 7+ messages in thread
From: Arend van Spriel @ 2017-06-21 21:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: backports
On 21-06-17 23:52, Johannes Berg wrote:
> On Wed, 2017-06-21 at 23:50 +0200, Arend van Spriel wrote:
>>
>>>> ++#if LINUX_VERSION_IS_LESS(4,13,0)
>>>> ++static void E(struct net_device *ndev)
>
>> So I will send a V3 fixing the macro in netdevice.h, but I don't
>> think we need the maybe_unused attribute.
>
> Ummm, yeah, I totally missed the #if above :)
>
> Maybe it's time to get some sleep...
Dito. That V3 will have to wait until tomorrow.
Gr. AvS
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2] backport: handle change in netdevice destructor usage
@ 2017-06-21 9:40 Arend van Spriel
0 siblings, 0 replies; 7+ messages in thread
From: Arend van Spriel @ 2017-06-21 9:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: backports, Arend van Spriel
This patch deals with changes made in struct net_device by commit
cf124db566e6 ("net: Fix inconsistent teardown and release of private
netdev state.").
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Hi Johannes,
I took a while for me to be happy with this version. More so because I
found that brcmfmac has a double-free bug if register_netdevice() fails
so I needed to solve that puzzle. Below is the changelog.
Regards,
Arend
V2:
- changed netdev_set_priv_destructor() to macro.
- revised semantic patch covering drivers only needing free_netdev().
- used some tricks suggested by Julia in semantic patch.
---
backport/backport-include/linux/netdevice.h | 9 ++++
patches/0079-netdev-destructor.cocci | 84 +++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
create mode 100644 patches/0079-netdev-destructor.cocci
diff --git a/backport/backport-include/linux/netdevice.h b/backport/backport-include/linux/netdevice.h
index 06230b5..0f5b198 100644
--- a/backport/backport-include/linux/netdevice.h
+++ b/backport/backport-include/linux/netdevice.h
@@ -320,4 +320,13 @@ static inline void netif_trans_update(struct net_device *dev)
}
#endif
+#if LINUX_VERSION_IS_LESS(4,13,0)
+#define netdev_set_priv_destructor(_dev, _destructor) \
+ (_dev)->destructor = __ ## _destructor
+#else
+ dev->needs_free_netdev = true;
+ if (destructor != free_netdev)
+ dev->priv_destructor = destructor;
+#endif
+
#endif /* __BACKPORT_NETDEVICE_H */
diff --git a/patches/0079-netdev-destructor.cocci b/patches/0079-netdev-destructor.cocci
new file mode 100644
index 0000000..fab8af1
--- /dev/null
+++ b/patches/0079-netdev-destructor.cocci
@@ -0,0 +1,84 @@
+@r1@
+struct net_device *NDEV;
+identifier D, C;
+identifier TRUE =~ "true";
+@@
+C(...)
+{
+ <...
+- NDEV->needs_free_netdev = TRUE;
+- NDEV->priv_destructor = D;
++ netdev_set_priv_destructor(NDEV, D);
+ ...>
+}
+
+@r2 depends on r1@
+identifier r1.D, r1.C;
+fresh identifier E = "__" ## D;
+@@
+
++#if LINUX_VERSION_IS_LESS(4,13,0)
++static void E(struct net_device *ndev)
++{
++ D(ndev);
++ free_netdev(ndev);
++}
++#endif
++
+C(...)
+{
+ ...
+}
+
+@r3 depends on r1@
+type T;
+identifier NDEV;
+identifier r1.D;
+T RET;
+@@
+
+RET = \(register_netdevice\|register_ndev\)(NDEV);
+if (<+... RET ...+>) {
+ <...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++ D(NDEV);
++#endif
+ free_netdev(NDEV);
+ ...>
+}
+
+@r4 depends on r1@
+identifier NDEV;
+identifier r1.D;
+type T;
+T RET;
+@@
+
+if (...)
+ RET = register_netdevice(NDEV);
+else
+ RET = register_netdev(NDEV);
+if (<+... RET ...+>) {
+ <...
++#if LINUX_VERSION_IS_LESS(4,13,0)
++ D(NDEV);
++#endif
+ free_netdev(NDEV);
+ ...>
+}
+
+@r5@
+struct net_device *NDEV;
+identifier TRUE =~ "true";
+@@
+
+-NDEV->needs_free_netdev = TRUE;
++netdev_set_priv_destructor(NDEV, free_netdev);
+
+@r6@
+struct net_device *NDEV;
+identifier D;
+@@
+
+-NDEV->priv_destructor = D;
++netdev_set_priv_destructor(NDEV, D);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe backports" in
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-21 21:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-21 20:58 [PATCH V2] backport: handle change in netdevice destructor usage Arend van Spriel
2017-06-21 21:17 ` Johannes Berg
2017-06-21 21:18 ` Johannes Berg
2017-06-21 21:50 ` Arend van Spriel
2017-06-21 21:52 ` Johannes Berg
2017-06-21 21:53 ` Arend van Spriel
-- strict thread matches above, loose matches on Subject: below --
2017-06-21 9:40 Arend van Spriel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.