| Message ID | 20250112140911.1702-3-ryazanov.s.a@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Hauke Mehrtens |
| Headers | show |
| Series | ipq40xx: fritzbox 7530: fix ADSL/ATM support | expand |
On 1/12/25 15:09, Sergey Ryazanov wrote: > ATM TC layer have some issues which effectively prevent VRX518 from > being used as ADSL modem. Specifically, there one crash during the ATM > layer configuration and wrong PVC ID selection on packet receiving what > breaks RX path. Fix both of the issues. Make subif iface registration > optional to prevent the crash (see more details in the new patch) and > update the hardcoded PVC ID to match the first allocated channel. > > Run tested with FRITZ!Box 7530. > > Fixes: 474bbe23b7 ("kernel: add Intel/Lantiq VRX518 TC driver") > Reported-and-tested-by: nebibigon93@yandex.ru > Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> > --- > .../lantiq/vrx518_tc/patches/100-compat.patch | 2 +- > ...tm_tc-fix-crash-on-subif_reg-absence.patch | 75 +++++++++++++++++++ > 2 files changed, 76 insertions(+), 1 deletion(-) > create mode 100644 package/kernel/lantiq/vrx518_tc/patches/207-dcdp-atm_tc-fix-crash-on-subif_reg-absence.patch > > diff --git a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch > index d04c1ed5df..81e32369ba 100644 > --- a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch > +++ b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch > @@ -166,7 +166,7 @@ > > - return (skb->DW0 >> 3) & 0xF; > +// return (skb->DW0 >> 3) & 0xF; > -+ return 1; > ++ return 0; /* We use only one connection for now, so return the first connection id */ > } > > static int atm_get_qid_by_vcc(struct net_device *dev, struct sk_buff *skb, Do we have the information atm_get_pvc_id() should return somewhere else in the skb? The DW0 attribute is the patched in DMA descriptor used to transfer this packet. Hauke
On 18.01.2025 22:18, Hauke Mehrtens wrote: > On 1/12/25 15:09, Sergey Ryazanov wrote: >> ATM TC layer have some issues which effectively prevent VRX518 from >> being used as ADSL modem. Specifically, there one crash during the ATM >> layer configuration and wrong PVC ID selection on packet receiving what >> breaks RX path. Fix both of the issues. Make subif iface registration >> optional to prevent the crash (see more details in the new patch) and >> update the hardcoded PVC ID to match the first allocated channel. >> >> Run tested with FRITZ!Box 7530. >> >> Fixes: 474bbe23b7 ("kernel: add Intel/Lantiq VRX518 TC driver") >> Reported-and-tested-by: nebibigon93@yandex.ru >> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> >> --- >> .../lantiq/vrx518_tc/patches/100-compat.patch | 2 +- >> ...tm_tc-fix-crash-on-subif_reg-absence.patch | 75 +++++++++++++++++++ >> 2 files changed, 76 insertions(+), 1 deletion(-) >> create mode 100644 package/kernel/lantiq/vrx518_tc/patches/207-dcdp- >> atm_tc-fix-crash-on-subif_reg-absence.patch >> >> diff --git a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch >> b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch >> index d04c1ed5df..81e32369ba 100644 >> --- a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch >> +++ b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch >> @@ -166,7 +166,7 @@ >> - return (skb->DW0 >> 3) & 0xF; >> +// return (skb->DW0 >> 3) & 0xF; >> -+ return 1; >> ++ return 0; /* We use only one connection for now, so return >> the first connection id */ >> } >> static int atm_get_qid_by_vcc(struct net_device *dev, struct >> sk_buff *skb, > > Do we have the information atm_get_pvc_id() should return somewhere else > in the skb? At the ATM/PTM TC level, skb lacks any metadata at the moment. We build skb in rxout_action() in sw_plat.c and copy only the actual data in it. Technically, we have one unclear field in the rx descriptor called 'qid' which is commented as 'DW 0'. Maybe we can reverse engineer its format and use it to map a received packet to a specific PVC. Suddenly, I do not have access to that device anymore and unlikely can have it a reasonable time. And cannot verify any guess. We can leave a fully functional mapping for a future development. In a worst case scenario we will receive a bug report from someone who needs an extra PVC for e.g. IPTV. But receives all the IPTV packets on the first (data) PVC. > The DW0 attribute is the patched in DMA descriptor used to transfer this > packet. Cannot follow you here. Could you clarify at bit? -- Sergey
diff --git a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch index d04c1ed5df..81e32369ba 100644 --- a/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch +++ b/package/kernel/lantiq/vrx518_tc/patches/100-compat.patch @@ -166,7 +166,7 @@ - return (skb->DW0 >> 3) & 0xF; +// return (skb->DW0 >> 3) & 0xF; -+ return 1; ++ return 0; /* We use only one connection for now, so return the first connection id */ } static int atm_get_qid_by_vcc(struct net_device *dev, struct sk_buff *skb, diff --git a/package/kernel/lantiq/vrx518_tc/patches/207-dcdp-atm_tc-fix-crash-on-subif_reg-absence.patch b/package/kernel/lantiq/vrx518_tc/patches/207-dcdp-atm_tc-fix-crash-on-subif_reg-absence.patch new file mode 100644 index 0000000000..87456424c3 --- /dev/null +++ b/package/kernel/lantiq/vrx518_tc/patches/207-dcdp-atm_tc-fix-crash-on-subif_reg-absence.patch @@ -0,0 +1,75 @@ +From: Sergey Ryazanov <ryazanov.s.a@gmail.com> +Date: Fri, 10 Jan 2025 00:57:27 +0000 +Subject: [PATCH] vrx518_tc: atm_tc: fix crash on subif_reg absence + +VRX518 (sw_plat) platform does not provid the subif_reg/subif_unreg ops +in the same time ATM TC layer unconditionally calls them, what leads to +the kernel crash on the atm_hook_mpoa_setup hook invocation from the ATM +stack: + + vrx518_tc:mpoa_setup_sync : sync: conn: 0, vpi: 0, vci: 35, mpoa_type: 0, mpoa_mode: 0 + Unable to handle kernel NULL pointer dereference at virtual address 00000000 + +Subif registration is optional and PTM TC do this only when the +corresponding ops are defined. Do the same for ATM TC and call +subif_reg/subif_unreg only if they are not NULL. + +While at it, move subif related data preparation under the 'if' block +in order to group and isolate that aux code. + +Run tested with FRITZ!Box 7530. + +Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> +--- +--- a/dcdp/atm_tc.c ++++ b/dcdp/atm_tc.c +@@ -1232,8 +1232,9 @@ static void ppe_close(struct atm_vcc *vc + validate_oam_htu_entry(priv, 0); + spin_unlock_bh(&priv->atm_lock); + +- priv->tc_priv->tc_ops.subif_unreg(dev, (!dev) ? dev_name : dev->name, +- priv->conn[cid].subif_id, 0); ++ if (priv->tc_priv->tc_ops.subif_unreg) ++ priv->tc_priv->tc_ops.subif_unreg(dev, (!dev) ? dev_name : dev->name, ++ priv->conn[cid].subif_id, 0); + + memset(conn, 0, sizeof(*conn)); + +@@ -2791,24 +2792,26 @@ static void mpoa_setup_sync(struct atm_p + struct wtx_queue_config_t tx_qcfg; + struct uni_cell_header *cell_header; + struct atm_vcc *vcc; +- struct net_device *dev; +- char dev_name[32]; + + tc_dbg(priv->tc_priv, MSG_INIT, + "sync: conn: %d, vpi: %d, vci: %d, mpoa_type: %d, mpoa_mode: %d\n", + conn, priv->conn[conn].vcc->vpi, priv->conn[conn].vcc->vci, + priv->conn[conn].mpoa_type, priv->conn[conn].mpoa_mode); + +- dev = priv->conn[conn].dev; ++ if (priv->tc_priv->tc_ops.subif_reg) { ++ struct net_device *dev; ++ char dev_name[32]; ++ ++ dev = priv->conn[conn].dev; ++ if (!dev) ++ sprintf(dev_name, "atm_%d%d", ++ priv->conn[conn].vcc->vpi, priv->conn[conn].vcc->vci); + +- if (!dev) +- sprintf(dev_name, "atm_%d%d", +- priv->conn[conn].vcc->vpi, priv->conn[conn].vcc->vci); +- +- priv->tc_priv->tc_ops.subif_reg(dev, (!dev) ? dev_name : dev->name, +- &priv->conn[conn].subif_id, 0); +- tc_dbg(priv->tc_priv, MSG_INIT, +- "conn[%d]subif_id[%x]", conn, priv->conn[conn].subif_id); ++ priv->tc_priv->tc_ops.subif_reg(dev, !dev ? dev_name : dev->name, ++ &priv->conn[conn].subif_id, 0); ++ tc_dbg(priv->tc_priv, MSG_INIT, ++ "conn[%d]subif_id[%x]", conn, priv->conn[conn].subif_id); ++ } + vcc = priv->conn[conn].vcc; + + /* set htu entry */
ATM TC layer have some issues which effectively prevent VRX518 from being used as ADSL modem. Specifically, there one crash during the ATM layer configuration and wrong PVC ID selection on packet receiving what breaks RX path. Fix both of the issues. Make subif iface registration optional to prevent the crash (see more details in the new patch) and update the hardcoded PVC ID to match the first allocated channel. Run tested with FRITZ!Box 7530. Fixes: 474bbe23b7 ("kernel: add Intel/Lantiq VRX518 TC driver") Reported-and-tested-by: nebibigon93@yandex.ru Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> --- .../lantiq/vrx518_tc/patches/100-compat.patch | 2 +- ...tm_tc-fix-crash-on-subif_reg-absence.patch | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 package/kernel/lantiq/vrx518_tc/patches/207-dcdp-atm_tc-fix-crash-on-subif_reg-absence.patch