[PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Dmitry Torokhov]

The commit 4a336a23d619 ("kobject: copy env blob in one go") optimized
constructing uevent data for delivery over netlink by using the raw
environment buffer, instead of reconstructing it from individual
environment pointers. Unfortunately in doing so it broke suppressing
MODALIAS attribute for KOBJ_UNBIND events, as the code that suppressed this
attribute only adjusted the environment pointers, but left the buffer
itself alone. Let's fix it by making sure the offending attribute is
obliterated form the buffer as well.

Reported-by: Tariq Toukan <tariqt@mellanox.com>
Reported-by: Casey Leedom <leedom@chelsio.com>
Fixes: 4a336a23d619 ("kobject: copy env blob in one go")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 lib/kobject_uevent.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index f237a09a5862..ae47cc732ba6 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -297,7 +297,8 @@ static void cleanup_uevent_env(struct subprocess_info *info)
 static void zap_modalias_env(struct kobj_uevent_env *env)
 {
 	static const char modalias_prefix[] = "MODALIAS=";
-	int i;
+	size_t len;
+	int i, j;
 
 	for (i = 0; i < env->envp_idx;) {
 		if (strncmp(env->envp[i], modalias_prefix,
@@ -306,11 +307,18 @@ static void zap_modalias_env(struct kobj_uevent_env *env)
 			continue;
 		}
 
-		if (i != env->envp_idx - 1)
-			memmove(&env->envp[i], &env->envp[i + 1],
-				sizeof(env->envp[i]) * env->envp_idx - 1);
+		len = strlen(env->envp[i]) + 1;
+
+		if (i != env->envp_idx - 1) {
+			memmove(env->envp[i], env->envp[i + 1],
+				env->buflen - len);
+
+			for (j = i; j < env->envp_idx - 1; j++)
+				env->envp[j] = env->envp[j + 1] - len;
+		}
 
 		env->envp_idx--;
+		env->buflen -= len;
 	}
 }
 
-- 
2.15.1.504.g5279b80103-goog


-- 
Dmitry

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

  Thanks Dmitry!  I've asked Komali to test it against the problem that she reported internally.  She or I will get back to you on that testing ASAP.

  I see that we've now moved on to 4.15 and the problem first showed up in 4.14.  Once the fix is verified, should we see if it can get pushed back into the 4.14 stream?

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Dmitry Torokhov]

On Thu, Dec 14, 2017 at 12:15 PM, Casey Leedom <leedom@chelsio.com> wrote:
>   Thanks Dmitry!  I've asked Komali to test it against the problem that she reported internally.  She or I will get back to you on that testing ASAP.
>
>   I see that we've now moved on to 4.15 and the problem first showed up in 4.14.  Once the fix is verified, should we see if it can get pushed back into the 4.14 stream?
>

As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
think we need this patch in .14.

Thanks.

-- 
Dmitry

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

| From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
| Sent: Thursday, December 14, 2017 12:32 PM
| 
| As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
| think we need this patch in .14.

  Except that Komali's bisect showed that the bug occured with kernel.org:
1455cf8dbfd06aa7651dcfccbadb7a093944ca65 which went in on July 19, 2017 which looks like it may even have made it into 4.13 ...

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Dmitry Torokhov]

On Thu, Dec 14, 2017 at 1:38 PM, Casey Leedom <leedom@chelsio.com> wrote:
> | From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> | Sent: Thursday, December 14, 2017 12:32 PM
> |
> | As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
> | think we need this patch in .14.
>
>   Except that Komali's bisect showed that the bug occured with kernel.org:
> 1455cf8dbfd06aa7651dcfccbadb7a093944ca65 which went in on July 19, 2017 which looks like it may even have made it into 4.13 ...

No, it is not in 4.13, but it is in 4.14:

dtor@dtor-ws:~/kernel/linus $ git describe --contains
1455cf8dbfd06aa7651dcfccbadb7a093944ca65
v4.14-rc1~147^2~16^2~6

This commit was causing issues because of unfortunate udev rules that
tried to load modules on any uevent except "remove" (we've fixed it to
only act on "add" now). We also added workaround for it:

dtor@dtor-ws:~/kernel/linus $ git describe --contains
6878e7de6af726de47f9f3bec649c3f49e786586
v4.14-rc4~36^2~4

Unfortunately that workaround got broken with Eric's refactoring that
went into 4.15 and is being fixed with this patch.

Hope this helps.

-- 
Dmitry

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

  Ah, okay, I see.  So it's been getting "broken" and "fixed" for the last six months or so ... :-)  I think it's still broken in the latest 4.14.6 but I'll have to check with Komali on that.  I'm not 100% sure it's important enough to push back into 4.14.{x} if it is still broken there ...

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

Hi Dmitry,

  Komali reports that she still sees the originally reported issue with your
latest patch.  This issue started with kernel.org commit
1455cf8dbfd06aa7651dcfccbadb7a093944ca65 (Wed Jul 19 17:24:30 2017 -0700).

  cxgb4vf is getting loaded in the Hypervisor when a cxgb4 Virtual Function
is assigned to a Virtual Machine, and the VM is brought up.  We may need to
do some debuging ...  How would you like to proceed?

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Dmitry Torokhov]

On Fri, Dec 15, 2017 at 10:17 AM, Casey Leedom <leedom@chelsio.com> wrote:
> Hi Dmitry,
>
>   Komali reports that she still sees the originally reported issue with your
> latest patch.  This issue started with kernel.org commit
> 1455cf8dbfd06aa7651dcfccbadb7a093944ca65 (Wed Jul 19 17:24:30 2017 -0700).
>
>   cxgb4vf is getting loaded in the Hypervisor when a cxgb4 Virtual Function
> is assigned to a Virtual Machine, and the VM is brought up.  We may need to
> do some debuging ...  How would you like to proceed?

Hmm, can she collect output of 'udevadm monitor -p' at the time you
assign the adapter to the VM? Also, do you have any udev rules that is
specific to cxgb? Or any other custom udev rules?

Also, we only suppress the modalias string on driver unbind, could it
be that for you module loading is happening when driver is bound to
the device? Actually, as far as I can see, you share PCI IDs between
cxgb4 and cxgb4vf drivers, so they have similar modalias entries. How
do you normally decide what driver is to be loaded for a given device?

Thanks.

-- 
Dmitry

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

| From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
| Sent: Friday, December 15, 2017 10:53 AM
|
|  Hmm, can she collect output of 'udevadm monitor -p' at the time you
| assign the adapter to the VM?

Sure.  I'll have Komali report on that.

| Also, do you have any udev rules that is specific to cxgb? Or any other
| custom udev rules?

I don't ~think~ so, but I'm not super familiar with udev stuff.  Point me at
a place to look and I can get this information for you.  [[ Komali is
hopefully at home in bed sound asleep looking forward to a fun weekend of
adventures! ]]

| Also, we only suppress the modalias string on driver unbind, could it be
| that for you module loading is happening when driver is bound to the
| device?

What Komali is saying is that when she first instantiates the SR-IOV Virtual
Functions, cxgb4vf gets automatically loaded as the VFs come into existence.
This has been happening forever and the only way to prevent this from
happening is to blacklist the cxgb4vf module.  That's fine and normal
behavior and she manually does a "rmmod cxgb4vf" to get rid of it.

But now, after doing the "rmmod cxgb4vf", when she assigns a VF to a Virtual
Machine and brings the VM up, cxgb4vf gets reloaded again.  This is the new
behavior which got introduced with kernel.org commit
1455cf8dbfd06aa7651dcfccbadb7a093944ca65 ...

| Actually, as far as I can see, you share PCI IDs between cxgb4 and cxgb4vf
| drivers, so they have similar modalias entries. How do you normally decide
| what driver is to be loaded for a given device?

The PCI Device IDs for the VFs aren't quite the same as those for the base
adapter Physical Functions.  Our PCI Device IDs look like:

  PF0..3          0x{T}0{DD}
  PF4             0x{T}4{DD}
  PF5             0x{T}5{DD}
  PF6             0x{T}6{DD}
  PF0..3/VF1..16  0x{T}8{DD}

  Where:
    {T} is 4, 5, 6, ... the adapter chip base
    {DD} is the adapter Device ID for that chip base implementation

So the VFs PCI Device IDs are in fact different from any of the PFs.

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Tariq Toukan]

On 14/12/2017 10:32 PM, Dmitry Torokhov wrote:
> On Thu, Dec 14, 2017 at 12:15 PM, Casey Leedom <leedom@chelsio.com> wrote:
>>    Thanks Dmitry!  I've asked Komali to test it against the problem that she reported internally.  She or I will get back to you on that testing ASAP.
>>
>>    I see that we've now moved on to 4.15 and the problem first showed up in 4.14.  Once the fix is verified, should we see if it can get pushed back into the 4.14 stream?
>>
> 
> As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
> think we need this patch in .14.
> 
Exactly. The issue I reported does not exist in 4.14.
I am going to verify the fix now.

Thanks Dmitry.

> Thanks.
> 

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Tariq Toukan]

On 17/12/2017 5:29 PM, Tariq Toukan wrote:
> 
> 
> On 14/12/2017 10:32 PM, Dmitry Torokhov wrote:
>> On Thu, Dec 14, 2017 at 12:15 PM, Casey Leedom <leedom@chelsio.com> 
>> wrote:
>>>    Thanks Dmitry!  I've asked Komali to test it against the problem 
>>> that she reported internally.  She or I will get back to you on that 
>>> testing ASAP.
>>>
>>>    I see that we've now moved on to 4.15 and the problem first showed 
>>> up in 4.14.  Once the fix is verified, should we see if it can get 
>>> pushed back into the 4.14 stream?
>>>
>>
>> As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
>> think we need this patch in .14.
>>
> Exactly. The issue I reported does not exist in 4.14.
> I am going to verify the fix now.
> 

Acked-by: Tariq Toukan <tariqt@mellanox.com>

Fix works, thanks.

> Thanks Dmitry.
> 
>> Thanks.
>>

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

/ From: Casey Leedom <leedom@chelsio.com>
| Date: Friday, December 15, 2017 11:17 PST
| 
| | From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
| | Sent: Friday, December 15, 2017 10:53 AM
| | 
| |  Hmm, can she collect output of 'udevadm monitor -p' at the time you
| | assign the adapter to the VM?
| 
| Sure.  I'll have Komali report on that.

  See appended.

| | Also, we only suppress the modalias string on driver unbind, could it
| | be that for you module loading is happening when driver is bound to
| | the device?
| 
| What Komali is saying is that when she first instantiates the SR-IOV
| Virtual Functions, cxgb4vf gets automatically loaded as the VFs come
| into existence.  This has been happening forever and the only way to
| prevent this from happening is to blacklist the cxgb4vf module.  That's
| fine and normal behavior and she manually does a "rmmod cxgb4vf" to get
| rid of it.
| 
| But now, after doing the "rmmod cxgb4vf", when she assigns a VF to a
| Virtual Machine and brings the VM up, cxgb4vf gets reloaded again.
| This is the new behavior which got introduced with kernel.org commit
\ 1455cf8dbfd06aa7651dcfccbadb7a093944ca65 ...


/ From: Tariq Toukan <tariqt@mellanox.com>
| Sent: Sunday, December 17, 2017 7:29 AM
| 
| On 14/12/2017 10:32 PM, Dmitry Torokhov wrote:
| >
| > On Thu, Dec 14, 2017 at 12:15 PM, Casey Leedom <leedom@chelsio.com> wrote:
| >>
| >>    Thanks Dmitry!  I've asked Komali to test it against the problem
| >> that she reported internally.  She or I will get back to you on that
| >> testing ASAP.
| >>
| >>   I see that we've now moved on to 4.15 and the problem first showed
| >> up in 4.14.  Once the fix is verified, should we see if it can get
| >> pushed back into the 4.14 stream?
| > 
| > As far as I can see Eric's patch is landed in 4.15-rc1 so I do not
| > think we need this patch in .14.
| 
| Exactly. The issue I reported does not exist in 4.14.
\ I am going to verify the fix now.

  Hhmmm.  Okay, I'll ask Komali to test this with the most complete bits
available for 4.14.7.  But it was my impression from talking to Komali that
this got broken back in July with kernel.org:1455cf8 and has been broken
ever since.  I suppose the bisect she did could have missed a brief point
where it wasn't broken.

Casey

-----

Komali's "udevadm monitor -p" output:

[root@t5nic ~]# udevadm monitor -p
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent

KERNEL[1326.743631] add      /devices/virtual/vfio/46 (vfio)
ACTION=add
DEVNAME=/dev/vfio/46
DEVPATH=/devices/virtual/vfio/46
MAJOR=241
MINOR=0
SEQNUM=8199
SUBSYSTEM=vfio

KERNEL[1326.743698] bind     /devices/pci0000:00/0000:00:03.0/0000:07:01.0 (pci)
ACTION=bind
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.0
DRIVER=vfio-pci
MODALIAS=pci:v00001425d00006801sv00001425sd00000000bc02sc00i00
PCI_CLASS=20000
PCI_ID=1425:6801
PCI_SLOT_NAME=0000:07:01.0
PCI_SUBSYS_ID=1425:0000
SEQNUM=8200
SUBSYSTEM=pci

UDEV  [1326.744244] add      /devices/virtual/vfio/46 (vfio)
ACTION=add
DEVNAME=/dev/vfio/46
DEVPATH=/devices/virtual/vfio/46
MAJOR=241
MINOR=0
SEQNUM=8199
SUBSYSTEM=vfio
USEC_INITIALIZED=26743678

KERNEL[1326.744570] add      /devices/virtual/net/macvtap0 (net)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0
IFINDEX=182
INTERFACE=macvtap0
SEQNUM=8201
SUBSYSTEM=net

KERNEL[1326.744584] add      /devices/virtual/net/macvtap0/queues/rx-0 (queues)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0/queues/rx-0
SEQNUM=8202
SUBSYSTEM=queues

KERNEL[1326.744592] add      /devices/virtual/net/macvtap0/queues/tx-0 (queues)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0/queues/tx-0
SEQNUM=8203
SUBSYSTEM=queues

KERNEL[1326.744773] add      /devices/virtual/net/macvtap0/macvtap/tap182 (macvtap)
ACTION=add
DEVNAME=/dev/tap182
DEVPATH=/devices/virtual/net/macvtap0/macvtap/tap182
MAJOR=240
MINOR=1
SEQNUM=8204
SUBSYSTEM=macvtap

UDEV  [1326.745942] add      /devices/virtual/net/macvtap0 (net)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0
ID_NET_DRIVER=macvlan
IFINDEX=182
INTERFACE=macvtap0
SEQNUM=8201
SUBSYSTEM=net
SYSTEMD_ALIAS=/sys/subsystem/net/devices/macvtap0
TAGS=:systemd:
USEC_INITIALIZED=744577

UDEV  [1326.746156] add      /devices/virtual/net/macvtap0/queues/rx-0 (queues)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0/queues/rx-0
SEQNUM=8202
SUBSYSTEM=queues
USEC_INITIALIZED=744631

UDEV  [1326.746497] add      /devices/virtual/net/macvtap0/queues/tx-0 (queues)
ACTION=add
DEVPATH=/devices/virtual/net/macvtap0/queues/tx-0
SEQNUM=8203
SUBSYSTEM=queues
USEC_INITIALIZED=26744640

UDEV  [1326.746664] add      /devices/virtual/net/macvtap0/macvtap/tap182 (macvtap)
ACTION=add
DEVNAME=/dev/tap182
DEVPATH=/devices/virtual/net/macvtap0/macvtap/tap182
MAJOR=240
MINOR=1
SEQNUM=8204
SUBSYSTEM=macvtap
USEC_INITIALIZED=26744774

KERNEL[1326.752714] add      /module/cxgb4vf (module)
ACTION=add
DEVPATH=/module/cxgb4vf
SEQNUM=8205
SUBSYSTEM=module

UDEV  [1326.752868] add      /module/cxgb4vf (module)
ACTION=add
DEVPATH=/module/cxgb4vf
SEQNUM=8205
SUBSYSTEM=module
SYSTEMD_WANTS=rdma.service
TAGS=:systemd:
USEC_INITIALIZED=752719

KERNEL[1326.813213] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0 (net)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0
IFINDEX=183
INTERFACE=eth0
SEQNUM=8209
SUBSYSTEM=net

KERNEL[1326.813236] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-0 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-0
SEQNUM=8210
SUBSYSTEM=queues

KERNEL[1326.813258] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-1 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-1
SEQNUM=8211
SUBSYSTEM=queues

KERNEL[1326.813271] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-2 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-2
SEQNUM=8212
SUBSYSTEM=queues

KERNEL[1326.813284] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-3 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-3
SEQNUM=8213
SUBSYSTEM=queues

KERNEL[1326.813298] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-0 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-0
SEQNUM=8214
SUBSYSTEM=queues

KERNEL[1326.813311] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-1 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-1
SEQNUM=8215
SUBSYSTEM=queues

KERNEL[1326.813323] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-2 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-2
SEQNUM=8216
SUBSYSTEM=queues

KERNEL[1326.813335] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-3 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-3
SEQNUM=8217
SUBSYSTEM=queues

KERNEL[1326.816577] move     /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4 (net)
ACTION=move
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4
DEVPATH_OLD=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0
IFINDEX=183
INTERFACE=enp7s1f4
SEQNUM=8218
SUBSYSTEM=net

KERNEL[1326.829318] bind     /devices/pci0000:00/0000:00:03.0/0000:07:01.4 (pci)
ACTION=bind
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4
DRIVER=cxgb4vf
MODALIAS=pci:v00001425d00006801sv00001425sd00000000bc02sc00i00
PCI_CLASS=20000
PCI_ID=1425:6801
PCI_SLOT_NAME=0000:07:01.4
PCI_SUBSYS_ID=1425:0000
SEQNUM=8219
SUBSYSTEM=pci

KERNEL[1326.829408] add      /bus/pci/drivers/cxgb4vf (drivers)
ACTION=add
DEVPATH=/bus/pci/drivers/cxgb4vf
SEQNUM=8220
SUBSYSTEM=drivers

UDEV  [1326.829723] bind     /devices/pci0000:00/0000:00:03.0/0000:07:01.0 (pci)
ACTION=bind
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.0
DRIVER=vfio-pci
ID_MODEL_FROM_DATABASE=T6225-CR Unified Wire Ethernet Controller [VF]
ID_PCI_CLASS_FROM_DATABASE=Network controller
ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
ID_VENDOR_FROM_DATABASE=Chelsio Communications Inc
MODALIAS=pci:v00001425d00006801sv00001425sd00000000bc02sc00i00
PCI_CLASS=20000
PCI_ID=1425:6801
PCI_SLOT_NAME=0000:07:01.0
PCI_SUBSYS_ID=1425:0000
SEQNUM=8200
SUBSYSTEM=pci
USEC_INITIALIZED=26743862

UDEV  [1326.829758] add      /bus/pci/drivers/cxgb4vf (drivers)
ACTION=add
DEVPATH=/bus/pci/drivers/cxgb4vf
SEQNUM=8220
SUBSYSTEM=drivers
USEC_INITIALIZED=829342

UDEV  [1326.843155] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4 (net)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4
ID_BUS=pci
ID_MODEL_FROM_DATABASE=T6225-CR Unified Wire Ethernet Controller [VF]
ID_MODEL_ID=0x6801
ID_NET_DRIVER=cxgb4vf
ID_NET_NAME_MAC=enx06443ca84001
ID_NET_NAME_PATH=enp7s1f4
ID_PATH=pci-0000:07:01.4
ID_PATH_TAG=pci-0000_07_01_4
ID_PCI_CLASS_FROM_DATABASE=Network controller
ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
ID_VENDOR_FROM_DATABASE=Chelsio Communications Inc
ID_VENDOR_ID=0x1425
IFINDEX=183
INTERFACE=enp7s1f4
INTERFACE_OLD=eth0
SEQNUM=8209
SUBSYSTEM=net
SYSTEMD_ALIAS=/sys/subsystem/net/devices/enp7s1f4
TAGS=:systemd:
USEC_INITIALIZED=813259

UDEV  [1326.843528] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-0 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-0
SEQNUM=8210
SUBSYSTEM=queues
USEC_INITIALIZED=813325

UDEV  [1326.843556] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-1 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-1
SEQNUM=8211
SUBSYSTEM=queues
USEC_INITIALIZED=813334

UDEV  [1326.843580] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-2 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-2
SEQNUM=8212
SUBSYSTEM=queues
USEC_INITIALIZED=813342

UDEV  [1326.843592] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-3 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/rx-3
SEQNUM=8213
SUBSYSTEM=queues
USEC_INITIALIZED=813349

UDEV  [1326.843729] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-0 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-0
SEQNUM=8214
SUBSYSTEM=queues
USEC_INITIALIZED=26813356

UDEV  [1326.843930] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-1 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-1
SEQNUM=8215
SUBSYSTEM=queues
USEC_INITIALIZED=26813364

UDEV  [1326.843966] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-2 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-2
SEQNUM=8216
SUBSYSTEM=queues
USEC_INITIALIZED=26813374

UDEV  [1326.844106] add      /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-3 (queues)
ACTION=add
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0/queues/tx-3
SEQNUM=8217
SUBSYSTEM=queues
USEC_INITIALIZED=26813381

UDEV  [1326.844576] move     /devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4 (net)
ACTION=move
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/enp7s1f4
DEVPATH_OLD=/devices/pci0000:00/0000:00:03.0/0000:07:01.4/net/eth0
ID_BUS=pci
ID_MODEL_FROM_DATABASE=T6225-CR Unified Wire Ethernet Controller [VF]
ID_MODEL_ID=0x6801
ID_NET_DRIVER=cxgb4vf
ID_NET_NAME_MAC=enx06443ca84001
ID_NET_NAME_PATH=enp7s1f4
ID_PATH=pci-0000:07:01.4
ID_PATH_TAG=pci-0000_07_01_4
ID_PCI_CLASS_FROM_DATABASE=Network controller
ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
ID_VENDOR_FROM_DATABASE=Chelsio Communications Inc
ID_VENDOR_ID=0x1425
IFINDEX=183
INTERFACE=enp7s1f4
SEQNUM=8218
SUBSYSTEM=net
SYSTEMD_ALIAS=/sys/subsystem/net/devices/enp7s1f4 /sys/subsystem/net/devices/enp7s1f4
TAGS=:systemd:
USEC_INITIALIZED=813259

UDEV  [1326.844939] bind     /devices/pci0000:00/0000:00:03.0/0000:07:01.4 (pci)
ACTION=bind
DEVPATH=/devices/pci0000:00/0000:00:03.0/0000:07:01.4
DRIVER=cxgb4vf
ID_MODEL_FROM_DATABASE=T6225-CR Unified Wire Ethernet Controller [VF]
ID_PCI_CLASS_FROM_DATABASE=Network controller
ID_PCI_SUBCLASS_FROM_DATABASE=Ethernet controller
ID_VENDOR_FROM_DATABASE=Chelsio Communications Inc
MODALIAS=pci:v00001425d00006801sv00001425sd00000000bc02sc00i00
PCI_CLASS=20000
PCI_ID=1425:6801
PCI_SLOT_NAME=0000:07:01.4
PCI_SUBSYS_ID=1425:0000
SEQNUM=8219
SUBSYSTEM=pci
USEC_INITIALIZED=829316

KERNEL[1326.876520] change   /devices/virtual/misc/kvm (misc)
ACTION=change
COUNT=1
CREATED=3
DEVNAME=/dev/kvm
DEVPATH=/devices/virtual/misc/kvm
EVENT=create
MAJOR=10
MINOR=232
PID=3873
SEQNUM=8221
STATS_PATH=/kvm/3873-11
SUBSYSTEM=misc

UDEV  [1326.877678] change   /devices/virtual/misc/kvm (misc)
ACTION=change
COUNT=1
CREATED=3
DEVNAME=/dev/kvm
DEVPATH=/devices/virtual/misc/kvm
EVENT=create
MAJOR=10
MINOR=232
PID=3873
SEQNUM=8221
STATS_PATH=/kvm/3873-11
SUBSYSTEM=misc
TAGS=:seat:uaccess:
USEC_INITIALIZED=648

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

  By the way, Komali just reported another bug internally where
instantiating an SR-IOV Virtual Function causes cxgb4vf to be automatically
loaded, which is normal behavior.  But then when she attempts to unload
cxgb4vf, it simply gets reloaded again.  This is with:

    [root@t5nic linux]#  git show
    commit 1291a0d5049dbc06baaaf66a9ff3f53db493b19b
    Author: Linus Torvalds <torvalds@linux-foundation.org>
    Date:   Sun Dec 17 18:59:59 2017 -0800

        Linux 4.15-rc4

    diff --git a/Makefile b/Makefile
    index 3f4d157..7e02f95 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -2,7 +2,7 @@
     VERSION = 4
     PATCHLEVEL = 15
     SUBLEVEL = 0
    -EXTRAVERSION = -rc3
    +EXTRAVERSION = -rc4
     NAME = Fearless Coyote
    ...

The patch you provided last Wednesday _does_ solve this new problem, but
doesn't solve the problem that she originally reported.  Let me know if
you'd like to see the udevadm output for 4.15-rc4 prior to applying your
patch.

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Greg Kroah-Hartman]

On Wed, Dec 13, 2017 at 03:21:22PM -0800, Dmitry Torokhov wrote:
> The commit 4a336a23d619 ("kobject: copy env blob in one go") optimized
> constructing uevent data for delivery over netlink by using the raw
> environment buffer, instead of reconstructing it from individual
> environment pointers. Unfortunately in doing so it broke suppressing
> MODALIAS attribute for KOBJ_UNBIND events, as the code that suppressed this
> attribute only adjusted the environment pointers, but left the buffer
> itself alone. Let's fix it by making sure the offending attribute is
> obliterated form the buffer as well.
> 
> Reported-by: Tariq Toukan <tariqt@mellanox.com>
> Reported-by: Casey Leedom <leedom@chelsio.com>
> Fixes: 4a336a23d619 ("kobject: copy env blob in one go")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  lib/kobject_uevent.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)

Ok, I'm lost here, is this patch still needed for 4.15-final?

And am I supposed to be taking it through my tree as the offending patch
didn't come from me?  :)

thanks,

greg k-h

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Eric Dumazet]

On Tue, Dec 19, 2017 at 12:48 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Dec 13, 2017 at 03:21:22PM -0800, Dmitry Torokhov wrote:
>> The commit 4a336a23d619 ("kobject: copy env blob in one go") optimized
>> constructing uevent data for delivery over netlink by using the raw
>> environment buffer, instead of reconstructing it from individual
>> environment pointers. Unfortunately in doing so it broke suppressing
>> MODALIAS attribute for KOBJ_UNBIND events, as the code that suppressed this
>> attribute only adjusted the environment pointers, but left the buffer
>> itself alone. Let's fix it by making sure the offending attribute is
>> obliterated form the buffer as well.
>>
>> Reported-by: Tariq Toukan <tariqt@mellanox.com>
>> Reported-by: Casey Leedom <leedom@chelsio.com>
>> Fixes: 4a336a23d619 ("kobject: copy env blob in one go")
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>  lib/kobject_uevent.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> Ok, I'm lost here, is this patch still needed for 4.15-final?
>
> And am I supposed to be taking it through my tree as the offending patch
> didn't come from me?  :)

Sorry for that, I sent a patch series (
8ca712c373a462cfa1b62272870b6c2c74aa83f9 )
that was mostly network related stuff (dealing with skbs and all rtnl batching),
and sent it to David before Dmitry patch was visible in David tree.

Yes, we believe fix in zap_modalias_env() is needed.

Thanks.

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Tariq Toukan]

On 19/12/2017 11:12 AM, Eric Dumazet wrote:
> On Tue, Dec 19, 2017 at 12:48 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Wed, Dec 13, 2017 at 03:21:22PM -0800, Dmitry Torokhov wrote:
>>> The commit 4a336a23d619 ("kobject: copy env blob in one go") optimized
>>> constructing uevent data for delivery over netlink by using the raw
>>> environment buffer, instead of reconstructing it from individual
>>> environment pointers. Unfortunately in doing so it broke suppressing
>>> MODALIAS attribute for KOBJ_UNBIND events, as the code that suppressed this
>>> attribute only adjusted the environment pointers, but left the buffer
>>> itself alone. Let's fix it by making sure the offending attribute is
>>> obliterated form the buffer as well.
>>>
>>> Reported-by: Tariq Toukan <tariqt@mellanox.com>
>>> Reported-by: Casey Leedom <leedom@chelsio.com>
>>> Fixes: 4a336a23d619 ("kobject: copy env blob in one go")
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>>   lib/kobject_uevent.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> Ok, I'm lost here, is this patch still needed for 4.15-final?
>>

Yes this is needed, it solves the issue I reported.
Thanks.

>> And am I supposed to be taking it through my tree as the offending patch
>> didn't come from me?  :)
> 
> Sorry for that, I sent a patch series (
> 8ca712c373a462cfa1b62272870b6c2c74aa83f9 )
> that was mostly network related stuff (dealing with skbs and all rtnl batching),
> and sent it to David before Dmitry patch was visible in David tree.
> 
> Yes, we believe fix in zap_modalias_env() is needed.
> 
> Thanks.
> 

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

  By the way, Komali went back and tried 4.14.7 and it does exhibit the
issue which she originally reported.  As noted before, the origin of the new
behavior was tracked down to kernel.org:1455cf8 ...  Also as noted, Dmitry's
patch does not solve the problem.

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Dmitry Torokhov]

On Tue, Dec 19, 2017 at 3:20 PM, Casey Leedom <leedom@chelsio.com> wrote:
>   By the way, Komali went back and tried 4.14.7 and it does exhibit the
> issue which she originally reported.  As noted before, the origin of the new
> behavior was tracked down to kernel.org:1455cf8 ...  Also as noted, Dmitry's
> patch does not solve the problem.

OK, so there is 2 separate issues. First is that the "zapping" of
modaliases was broken by Eric's changes and we do need the patch above
to fix it so we do not end up with modules being essentially
not-unloadable on systems with old udev rules, as since trying to
unload module would result in unbinding from a device, and udev would
see unbind uevent and immediately reload the module in question. Greg,
can you please schedule my patch in for 4.15?

The 2nd issue is that Casey is seeing cxgb4vf driver being loaded.
This happens because vfio-pci driver binds to the device, which causes
KOBJ_BIND to get emitted, which causes all modules that match the PCI
device ID that was bound to load [again].

While we could fix the issue by putting another bandaid and zapping
the modalias on KOBJ_BIND as well, I am not sure we need to do that,
as I do not see a harm in having cxgb4vf module loaded. Rather than
patching the kernel I'd recommend Casey simply update the udev scripts
to only load module on KOBJ_ADD event:
https://github.com/systemd/systemd/commit/9b32afa9f241fe8febc0a754850f1e7331caf6e3#diff-0e123aabb4420b8b95088699179f3416

Thanks.

-- 
Dmitry

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

| From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
| Sent: Wednesday, December 20, 2017 9:13 PM
|
| The 2nd issue is that Casey is seeing cxgb4vf driver being loaded.
| This happens because vfio-pci driver binds to the device, which causes
| KOBJ_BIND to get emitted, which causes all modules that match the PCI
| device ID that was bound to load [again].
|
| While we could fix the issue by putting another bandaid and zapping
| the modalias on KOBJ_BIND as well, I am not sure we need to do that,
| as I do not see a harm in having cxgb4vf module loaded. Rather than
| patching the kernel I'd recommend Casey simply update the udev scripts
| to only load module on KOBJ_ADD event:
|
| https://github.com/systemd/systemd/commit/9b32afa9f241fe8febc0a754850f1e7331caf6e3#diff-0e123aabb4420b8b95088699179f3416
|
|  https://avatars3.githubusercontent.com/u/1047950?s=200&v=4
|
| rules: load drivers only on "add"        systemd/systemd@9b32afa
| github.com
|
| Previously we were loading kernel modules on all device events save for
| "remove". With the introduction of KOBJ_BIND/KOBJ_UNBIND this causes issues,
| as driver modules that have devices bound to the...

Thanks Dmitry.

Now I understand why your kernel.org:1455cf8 changed this behavior.  It
added new BIND/UNBIND events and the current udev rules treated that as a
load event.

I'm not familiar with the format of /etc/udev/rules.d/ files.  Komali's
system doesn't have a 80-drivers.rules entry.  It looks like she can just
add the file with the diff to test this out.  (Although I'm not sure what
"tifm_sd" is ... it looks like a specific driver ...)

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Casey Leedom]

Dmitry,

  Komali has done some experiments with changing /lib/udev/rules.d/80-drivers.rules along the
lines you suggested and that seems to be the trick (I'm waiting for clarification from her).
What's the mechanism for getting that change committed to the various repositories?

  Also, your fix for the other problem which Komali reported (not being able to remove
cxgb4vf once loaded) hasn't hit Linis' main repository yet as far as I can see.  Should we
be doing something about that?

Casey

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Tariq Toukan]

On 21/12/2017 7:13 AM, Dmitry Torokhov wrote:
>  Greg,
> can you please schedule my patch in for 4.15?
> 

We must have it in, otherwise there will be a major degradation (many 
drivers won't be able to reload).
Greg?

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Greg Kroah-Hartman]

On Sun, Dec 31, 2017 at 12:28:02PM +0200, Tariq Toukan wrote:
> 
> 
> On 21/12/2017 7:13 AM, Dmitry Torokhov wrote:
> >  Greg,
> > can you please schedule my patch in for 4.15?
> > 
> 
> We must have it in, otherwise there will be a major degradation (many
> drivers won't be able to reload).
> Greg?

It's already queued up to get merged, please be patient, it is the
holidays...

greg k-h

Re: [PATCH] kobject: fix suppressing modalias in uevents delivered over netlink

[Tariq Toukan]

On 31/12/2017 12:39 PM, Greg Kroah-Hartman wrote:
> On Sun, Dec 31, 2017 at 12:28:02PM +0200, Tariq Toukan wrote:
>>
>>
>> On 21/12/2017 7:13 AM, Dmitry Torokhov wrote:
>>>   Greg,
>>> can you please schedule my patch in for 4.15?
>>>
>>
>> We must have it in, otherwise there will be a major degradation (many
>> drivers won't be able to reload).
>> Greg?
> 
> It's already queued up to get merged, please be patient, it is the
> holidays...
> 
> greg k-h
> 
Many thanks, and happy holidays!

Tariq