RE: [PATCH net-next v7 3/7] r8169: add support for phylink

quoted

+     if (jumbo_before != jumbo_after) {
+             unsigned long caps =
+ tp->phylink_config.mac_capabilities;
+
+             if (jumbo_after)
+                     caps &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+             else
+                     caps |= (MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+
+             phylink_update_mac_pause_capabilities(tp->phylink,
+ caps);

The name suggests it only changes pause. So i would expect the caps
parameter passed here to only cover MAC_SYM_PAUSE & MAC_ASYM_PAUSE.
So i don't see a need to look at tp->phylink_config.mac_capabilities.

In fact, I added a strict check inside phylink_update_mac_pause_capabilities() to prevent non-pause bits from being modified.

+	if (mac_pause & ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE)) {
+		phylink_err(pl, "Attempted to dynamically change non-pause MAC capabilities\n");
+		return;
+	}

But I think you are right. I will fix the caller in v2 to only pass the specific MAC_SYM_PAUSE and MAC_ASYM_PAUSE bits, which perfectly matches the function's name and its internal check.

+ if (jumbo_before != jumbo_after) {
+ 	unsigned long mac_pause = 0;
+
+	if (!jumbo_after)
+		mac_pause = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
+
+	phylink_update_mac_pause_capabilities(tp->phylink, mac_pause);
+ }

       Andrew