| Message ID | 20220328061402.16276-1-qianfanguijin@163.com |
|---|---|
| State | Superseded |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | [v1] net: eth-uclass: Fix eth_halt | expand |
On Mon, Mar 28, 2022 at 9:14 AM <qianfanguijin@163.com> wrote: > > From: qianfan Zhao <qianfanguijin@163.com> > > eth_device_priv maybe unaccessable after @stop handler due to eth device > is removed in @stop. Setting private data before @stop handler. > > This also fix data abort bug when run dhcp or tftp command via usbnet. > > Signed-off-by: qianfan Zhao <qianfanguijin@163.com> > --- > net/eth-uclass.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > index 58c308f332..c6eb1bc8f8 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -338,9 +338,14 @@ void eth_halt(void) > if (!priv || !priv->running) > return; > > - eth_get_ops(current)->stop(current); > - priv->state = ETH_STATE_PASSIVE; Why did you delete this line ? > + /* Make sure setting private data before @stop handler, it may remove > + * ethernet device and will cause @priv unaccessable. > + * eg: > + * usb_eth_stop -> usb_gadget_release -> device_remove > + */ > priv->running = false; > + > + eth_get_ops(current)->stop(current); > } > > int eth_is_active(struct udevice *dev) > -- > 2.17.1 >
在 2022/4/2 3:26, Ramon Fried 写道: > On Mon, Mar 28, 2022 at 9:14 AM <qianfanguijin@163.com> wrote: >> From: qianfan Zhao <qianfanguijin@163.com> >> >> eth_device_priv maybe unaccessable after @stop handler due to eth device >> is removed in @stop. Setting private data before @stop handler. >> >> This also fix data abort bug when run dhcp or tftp command via usbnet. >> >> Signed-off-by: qianfan Zhao <qianfanguijin@163.com> >> --- >> net/eth-uclass.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/net/eth-uclass.c b/net/eth-uclass.c >> index 58c308f332..c6eb1bc8f8 100644 >> --- a/net/eth-uclass.c >> +++ b/net/eth-uclass.c >> @@ -338,9 +338,14 @@ void eth_halt(void) >> if (!priv || !priv->running) >> return; >> >> - eth_get_ops(current)->stop(current); >> - priv->state = ETH_STATE_PASSIVE; > Why did you delete this line ? As we all konw priv->state can't set after @stop handler. But some eth driver such as drivers/net/ldpaa_eth/ldpaa_eth.c check @priv->state in @stop handler, do noting if state is ETH_STATE_PASSIVE. So I delete this. But I think it's a mistake when I check source code again. Should get private again after @stop handler and set state to ETH_STATE_PASSIVE if private data available. I will send new patch later. >> + /* Make sure setting private data before @stop handler, it may remove >> + * ethernet device and will cause @priv unaccessable. >> + * eg: >> + * usb_eth_stop -> usb_gadget_release -> device_remove >> + */ >> priv->running = false; >> + >> + eth_get_ops(current)->stop(current); >> } >> >> int eth_is_active(struct udevice *dev) >> -- >> 2.17.1 >>
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 58c308f332..c6eb1bc8f8 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -338,9 +338,14 @@ void eth_halt(void) if (!priv || !priv->running) return; - eth_get_ops(current)->stop(current); - priv->state = ETH_STATE_PASSIVE; + /* Make sure setting private data before @stop handler, it may remove + * ethernet device and will cause @priv unaccessable. + * eg: + * usb_eth_stop -> usb_gadget_release -> device_remove + */ priv->running = false; + + eth_get_ops(current)->stop(current); } int eth_is_active(struct udevice *dev)