* [PATCH 2/4] isdn/eicon: fix some message formatting errors
From: Nicolas Iooss @ 2016-10-29 12:56 UTC (permalink / raw)
To: Armin Schindler, Karsten Keil; +Cc: netdev, linux-kernel, Nicolas Iooss
In-Reply-To: <20161029125605.13610-1-nicolas.iooss_linux@m4x.org>
There are some inconsistent debug message formats in message.c. For
example,
dprintf("XDI CAPI: RC cancelled Id:0x02, Ch:%02x", e->Id, ch);
wrongly reports an ID of 2 and prints the entity ID as the channel ID.
There are also object pointers which are used instead of the IDs.
All these inconsistent formats have been found by adding __printf
attribute to myDbgPrint_...() functions (used by dbug()). As this makes
the compiler to also complain about using "%ld" with unsigned int values
(instead of "%u") and some other less-important format issues, this
patch does not add any __printf attribute.
This patch has only been compile-tested.
Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---
drivers/isdn/hardware/eicon/message.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/message.c b/drivers/isdn/hardware/eicon/message.c
index 1a1d99704fe6..7cafa34c3464 100644
--- a/drivers/isdn/hardware/eicon/message.c
+++ b/drivers/isdn/hardware/eicon/message.c
@@ -1059,7 +1059,7 @@ static void plci_remove(PLCI *plci)
}
if (plci->Sig.Id == 0xff)
{
- dbug(1, dprintf("D-channel X.25 plci->NL.Id:%0x", plci->NL.Id));
+ dbug(1, dprintf("D-channel X.25 plci->NL.Id:%02x", plci->NL.Id));
if (plci->NL.Id && !plci->nl_remove_id)
{
nl_req_ncci(plci, REMOVE, 0);
@@ -3109,7 +3109,7 @@ static byte data_b3_req(dword Id, word Number, DIVA_CAPI_ADAPTER *a,
Info = _WRONG_IDENTIFIER;
ncci = (word)(Id >> 16);
- dbug(1, dprintf("ncci=0x%x, plci=0x%x", ncci, plci));
+ dbug(1, dprintf("ncci=0x%x, plci=0x%x", ncci, plci->Id));
if (plci && ncci)
{
@@ -3325,7 +3325,7 @@ static byte select_b_req(dword Id, word Number, DIVA_CAPI_ADAPTER *a,
else
{
dbug(1, dprintf("select_b_req[%d],PLCI=0x%x,Tel=0x%x,NL=0x%x,appl=0x%x,sstate=0x%x",
- msg->length, plci->Id, plci->tel, plci->NL.Id, plci->appl, plci->SuppState));
+ msg->length, plci->Id, plci->tel, plci->NL.Id, appl->Id, plci->SuppState));
dbug(1, dprintf("PlciState=0x%x", plci->State));
for (i = 0; i < 7; i++) bp_parms[i].length = 0;
@@ -3910,7 +3910,7 @@ void callback(ENTITY *e)
if (no_cancel_rc && (a->FlowControlIdTable[ch] == e->Id) && e->Id) {
a->FlowControlIdTable[ch] = 0;
if ((rc == OK) && a->FlowControlSkipTable[ch]) {
- dbug(3, dprintf("XDI CAPI: RC cancelled Id:0x02, Ch:%02x", e->Id, ch));
+ dbug(3, dprintf("XDI CAPI: RC cancelled Id:%02x, Ch:%02x", e->Id, ch));
return;
}
}
@@ -9135,7 +9135,7 @@ static word AdvCodecSupport(DIVA_CAPI_ADAPTER *a, PLCI *plci, APPL *appl,
{
if (a->AdvSignalAppl != appl || a->AdvSignalPLCI)
{
- dbug(1, dprintf("AdvSigPlci=0x%x", a->AdvSignalPLCI));
+ dbug(1, dprintf("AdvSigPlci=0x%x", a->AdvSignalPLCI->Id));
return 0x2001; /* codec in use by another application */
}
if (plci != NULL)
--
2.10.1
^ permalink raw reply related
* [PATCH 3/4] isdn/eicon: add some __printf attributes
From: Nicolas Iooss @ 2016-10-29 12:56 UTC (permalink / raw)
To: Armin Schindler, Karsten Keil; +Cc: netdev, linux-kernel, Nicolas Iooss
In-Reply-To: <20161029125605.13610-1-nicolas.iooss_linux@m4x.org>
Add __printf attributes to some functions. This helps detecting errors
related to printf-formats at compile time.
When doing this, gcc reports some issues in debug.c. Fix them.
This patch has only been compile-tested.
Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---
drivers/isdn/hardware/eicon/debug.c | 129 +++++++++++++++++----------------
drivers/isdn/hardware/eicon/maintidi.c | 3 +-
drivers/isdn/hardware/eicon/platform.h | 2 +-
3 files changed, 68 insertions(+), 66 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/debug.c b/drivers/isdn/hardware/eicon/debug.c
index 576b7b4a3278..cd8d70e3292d 100644
--- a/drivers/isdn/hardware/eicon/debug.c
+++ b/drivers/isdn/hardware/eicon/debug.c
@@ -24,6 +24,7 @@ static word SuperTraceCreateReadReq(byte *P, const char *path);
static int diva_mnt_cmp_nmbr(const char *nmbr);
static void diva_free_dma_descriptor(IDI_CALL request, int nr);
static int diva_get_dma_descriptor(IDI_CALL request, dword *dma_magic);
+__printf(3, 4)
void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
static dword MaxDumpSize = 256;
@@ -1514,29 +1515,29 @@ static void diva_maint_state_change_notify(void *user_context,
}
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Ch = %lu",
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Ch = %u",
(int)modem->ChannelNumber);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Event = %lu", modem->Event);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Norm = %lu", modem->Norm);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Event = %u", modem->Event);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Norm = %u", modem->Norm);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Opts. = 0x%08x", modem->Options);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Tx = %lu Bps", modem->TxSpeed);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Rx = %lu Bps", modem->RxSpeed);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RT = %lu mSec",
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Tx = %u Bps", modem->TxSpeed);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Rx = %u Bps", modem->RxSpeed);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RT = %u mSec",
modem->RoundtripMsec);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Sr = %lu", modem->SymbolRate);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Sr = %u", modem->SymbolRate);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Rxl = %d dBm", modem->RxLeveldBm);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM El = %d dBm", modem->EchoLeveldBm);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM SNR = %lu dB", modem->SNRdb);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM MAE = %lu", modem->MAE);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM LRet = %lu",
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM SNR = %u dB", modem->SNRdb);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM MAE = %u", modem->MAE);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM LRet = %u",
modem->LocalRetrains);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RRet = %lu",
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RRet = %u",
modem->RemoteRetrains);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM LRes = %lu", modem->LocalResyncs);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RRes = %lu",
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM LRes = %u", modem->LocalResyncs);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM RRes = %u",
modem->RemoteResyncs);
if (modem->Event == 3) {
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Disc = %lu", modem->DiscReason);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM Disc = %u", modem->DiscReason);
}
}
if ((modem->Event == 3) && (pC->hDbg->dbgMask & DIVA_MGT_DBG_MDM_STATISTICS)) {
@@ -1560,20 +1561,20 @@ static void diva_maint_state_change_notify(void *user_context,
}
}
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Ch = %lu", (int)fax->ChannelNumber);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Event = %lu", fax->Event);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Pages = %lu", fax->Page_Counter);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Ch = %u", (int)fax->ChannelNumber);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Event = %u", fax->Event);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Pages = %u", fax->Page_Counter);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Feat. = 0x%08x", fax->Features);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX ID = <%s>", &fax->Station_ID[0]);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Saddr = <%s>", &fax->Subaddress[0]);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Pwd = <%s>", &fax->Password[0]);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Speed = %lu", fax->Speed);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Speed = %u", fax->Speed);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Res. = 0x%08x", fax->Resolution);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Width = %lu", fax->Paper_Width);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Length= %lu", fax->Paper_Length);
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX SLT = %lu", fax->Scanline_Time);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Width = %u", fax->Paper_Width);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Length= %u", fax->Paper_Length);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX SLT = %u", fax->Scanline_Time);
if (fax->Event == 3) {
- diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Disc = %lu", fax->Disc_Reason);
+ diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "FAX Disc = %u", fax->Disc_Reason);
}
}
if ((fax->Event == 3) && (pC->hDbg->dbgMask & DIVA_MGT_DBG_FAX_STATISTICS)) {
@@ -1597,35 +1598,35 @@ static void diva_maint_state_change_notify(void *user_context,
*/
if (channel->pInterfaceStat->inc.Calls) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Calls =%lu", channel->pInterfaceStat->inc.Calls);
+ "Inc Calls =%u", channel->pInterfaceStat->inc.Calls);
}
if (channel->pInterfaceStat->inc.Connected) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Connected =%lu", channel->pInterfaceStat->inc.Connected);
+ "Inc Connected =%u", channel->pInterfaceStat->inc.Connected);
}
if (channel->pInterfaceStat->inc.User_Busy) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Busy =%lu", channel->pInterfaceStat->inc.User_Busy);
+ "Inc Busy =%u", channel->pInterfaceStat->inc.User_Busy);
}
if (channel->pInterfaceStat->inc.Call_Rejected) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Rejected =%lu", channel->pInterfaceStat->inc.Call_Rejected);
+ "Inc Rejected =%u", channel->pInterfaceStat->inc.Call_Rejected);
}
if (channel->pInterfaceStat->inc.Wrong_Number) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Wrong Nr =%lu", channel->pInterfaceStat->inc.Wrong_Number);
+ "Inc Wrong Nr =%u", channel->pInterfaceStat->inc.Wrong_Number);
}
if (channel->pInterfaceStat->inc.Incompatible_Dst) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Incomp. Dest =%lu", channel->pInterfaceStat->inc.Incompatible_Dst);
+ "Inc Incomp. Dest =%u", channel->pInterfaceStat->inc.Incompatible_Dst);
}
if (channel->pInterfaceStat->inc.Out_of_Order) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Out of Order =%lu", channel->pInterfaceStat->inc.Out_of_Order);
+ "Inc Out of Order =%u", channel->pInterfaceStat->inc.Out_of_Order);
}
if (channel->pInterfaceStat->inc.Ignored) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Inc Ignored =%lu", channel->pInterfaceStat->inc.Ignored);
+ "Inc Ignored =%u", channel->pInterfaceStat->inc.Ignored);
}
/*
@@ -1633,31 +1634,31 @@ static void diva_maint_state_change_notify(void *user_context,
*/
if (channel->pInterfaceStat->outg.Calls) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Calls =%lu", channel->pInterfaceStat->outg.Calls);
+ "Outg Calls =%u", channel->pInterfaceStat->outg.Calls);
}
if (channel->pInterfaceStat->outg.Connected) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Connected =%lu", channel->pInterfaceStat->outg.Connected);
+ "Outg Connected =%u", channel->pInterfaceStat->outg.Connected);
}
if (channel->pInterfaceStat->outg.User_Busy) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Busy =%lu", channel->pInterfaceStat->outg.User_Busy);
+ "Outg Busy =%u", channel->pInterfaceStat->outg.User_Busy);
}
if (channel->pInterfaceStat->outg.No_Answer) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg No Answer =%lu", channel->pInterfaceStat->outg.No_Answer);
+ "Outg No Answer =%u", channel->pInterfaceStat->outg.No_Answer);
}
if (channel->pInterfaceStat->outg.Wrong_Number) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Wrong Nr =%lu", channel->pInterfaceStat->outg.Wrong_Number);
+ "Outg Wrong Nr =%u", channel->pInterfaceStat->outg.Wrong_Number);
}
if (channel->pInterfaceStat->outg.Call_Rejected) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Rejected =%lu", channel->pInterfaceStat->outg.Call_Rejected);
+ "Outg Rejected =%u", channel->pInterfaceStat->outg.Call_Rejected);
}
if (channel->pInterfaceStat->outg.Other_Failures) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "Outg Other Failures =%lu", channel->pInterfaceStat->outg.Other_Failures);
+ "Outg Other Failures =%u", channel->pInterfaceStat->outg.Other_Failures);
}
}
break;
@@ -1665,114 +1666,114 @@ static void diva_maint_state_change_notify(void *user_context,
case DIVA_SUPER_TRACE_NOTIFY_MDM_STAT_CHANGE:
if (channel->pInterfaceStat->mdm.Disc_Normal) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Normal = %lu", channel->pInterfaceStat->mdm.Disc_Normal);
+ "MDM Disc Normal = %u", channel->pInterfaceStat->mdm.Disc_Normal);
}
if (channel->pInterfaceStat->mdm.Disc_Unspecified) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Unsp. = %lu", channel->pInterfaceStat->mdm.Disc_Unspecified);
+ "MDM Disc Unsp. = %u", channel->pInterfaceStat->mdm.Disc_Unspecified);
}
if (channel->pInterfaceStat->mdm.Disc_Busy_Tone) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Busy Tone = %lu", channel->pInterfaceStat->mdm.Disc_Busy_Tone);
+ "MDM Disc Busy Tone = %u", channel->pInterfaceStat->mdm.Disc_Busy_Tone);
}
if (channel->pInterfaceStat->mdm.Disc_Congestion) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Congestion = %lu", channel->pInterfaceStat->mdm.Disc_Congestion);
+ "MDM Disc Congestion = %u", channel->pInterfaceStat->mdm.Disc_Congestion);
}
if (channel->pInterfaceStat->mdm.Disc_Carr_Wait) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Carrier Wait = %lu", channel->pInterfaceStat->mdm.Disc_Carr_Wait);
+ "MDM Disc Carrier Wait = %u", channel->pInterfaceStat->mdm.Disc_Carr_Wait);
}
if (channel->pInterfaceStat->mdm.Disc_Trn_Timeout) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Trn. T.o. = %lu", channel->pInterfaceStat->mdm.Disc_Trn_Timeout);
+ "MDM Disc Trn. T.o. = %u", channel->pInterfaceStat->mdm.Disc_Trn_Timeout);
}
if (channel->pInterfaceStat->mdm.Disc_Incompat) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Incompatible = %lu", channel->pInterfaceStat->mdm.Disc_Incompat);
+ "MDM Disc Incompatible = %u", channel->pInterfaceStat->mdm.Disc_Incompat);
}
if (channel->pInterfaceStat->mdm.Disc_Frame_Rej) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc Frame Reject = %lu", channel->pInterfaceStat->mdm.Disc_Frame_Rej);
+ "MDM Disc Frame Reject = %u", channel->pInterfaceStat->mdm.Disc_Frame_Rej);
}
if (channel->pInterfaceStat->mdm.Disc_V42bis) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "MDM Disc V.42bis = %lu", channel->pInterfaceStat->mdm.Disc_V42bis);
+ "MDM Disc V.42bis = %u", channel->pInterfaceStat->mdm.Disc_V42bis);
}
break;
case DIVA_SUPER_TRACE_NOTIFY_FAX_STAT_CHANGE:
if (channel->pInterfaceStat->fax.Disc_Normal) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Normal = %lu", channel->pInterfaceStat->fax.Disc_Normal);
+ "FAX Disc Normal = %u", channel->pInterfaceStat->fax.Disc_Normal);
}
if (channel->pInterfaceStat->fax.Disc_Not_Ident) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Not Ident. = %lu", channel->pInterfaceStat->fax.Disc_Not_Ident);
+ "FAX Disc Not Ident. = %u", channel->pInterfaceStat->fax.Disc_Not_Ident);
}
if (channel->pInterfaceStat->fax.Disc_No_Response) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc No Response = %lu", channel->pInterfaceStat->fax.Disc_No_Response);
+ "FAX Disc No Response = %u", channel->pInterfaceStat->fax.Disc_No_Response);
}
if (channel->pInterfaceStat->fax.Disc_Retries) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Max Retries = %lu", channel->pInterfaceStat->fax.Disc_Retries);
+ "FAX Disc Max Retries = %u", channel->pInterfaceStat->fax.Disc_Retries);
}
if (channel->pInterfaceStat->fax.Disc_Unexp_Msg) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Unexp. Msg. = %lu", channel->pInterfaceStat->fax.Disc_Unexp_Msg);
+ "FAX Unexp. Msg. = %u", channel->pInterfaceStat->fax.Disc_Unexp_Msg);
}
if (channel->pInterfaceStat->fax.Disc_No_Polling) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc No Polling = %lu", channel->pInterfaceStat->fax.Disc_No_Polling);
+ "FAX Disc No Polling = %u", channel->pInterfaceStat->fax.Disc_No_Polling);
}
if (channel->pInterfaceStat->fax.Disc_Training) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Training = %lu", channel->pInterfaceStat->fax.Disc_Training);
+ "FAX Disc Training = %u", channel->pInterfaceStat->fax.Disc_Training);
}
if (channel->pInterfaceStat->fax.Disc_Unexpected) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Unexpected = %lu", channel->pInterfaceStat->fax.Disc_Unexpected);
+ "FAX Disc Unexpected = %u", channel->pInterfaceStat->fax.Disc_Unexpected);
}
if (channel->pInterfaceStat->fax.Disc_Application) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Application = %lu", channel->pInterfaceStat->fax.Disc_Application);
+ "FAX Disc Application = %u", channel->pInterfaceStat->fax.Disc_Application);
}
if (channel->pInterfaceStat->fax.Disc_Incompat) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Incompatible = %lu", channel->pInterfaceStat->fax.Disc_Incompat);
+ "FAX Disc Incompatible = %u", channel->pInterfaceStat->fax.Disc_Incompat);
}
if (channel->pInterfaceStat->fax.Disc_No_Command) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc No Command = %lu", channel->pInterfaceStat->fax.Disc_No_Command);
+ "FAX Disc No Command = %u", channel->pInterfaceStat->fax.Disc_No_Command);
}
if (channel->pInterfaceStat->fax.Disc_Long_Msg) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Long Msg. = %lu", channel->pInterfaceStat->fax.Disc_Long_Msg);
+ "FAX Disc Long Msg. = %u", channel->pInterfaceStat->fax.Disc_Long_Msg);
}
if (channel->pInterfaceStat->fax.Disc_Supervisor) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Supervisor = %lu", channel->pInterfaceStat->fax.Disc_Supervisor);
+ "FAX Disc Supervisor = %u", channel->pInterfaceStat->fax.Disc_Supervisor);
}
if (channel->pInterfaceStat->fax.Disc_SUB_SEP_PWD) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc SUP SEP PWD = %lu", channel->pInterfaceStat->fax.Disc_SUB_SEP_PWD);
+ "FAX Disc SUP SEP PWD = %u", channel->pInterfaceStat->fax.Disc_SUB_SEP_PWD);
}
if (channel->pInterfaceStat->fax.Disc_Invalid_Msg) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Invalid Msg. = %lu", channel->pInterfaceStat->fax.Disc_Invalid_Msg);
+ "FAX Disc Invalid Msg. = %u", channel->pInterfaceStat->fax.Disc_Invalid_Msg);
}
if (channel->pInterfaceStat->fax.Disc_Page_Coding) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Page Coding = %lu", channel->pInterfaceStat->fax.Disc_Page_Coding);
+ "FAX Disc Page Coding = %u", channel->pInterfaceStat->fax.Disc_Page_Coding);
}
if (channel->pInterfaceStat->fax.Disc_App_Timeout) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Appl. T.o. = %lu", channel->pInterfaceStat->fax.Disc_App_Timeout);
+ "FAX Disc Appl. T.o. = %u", channel->pInterfaceStat->fax.Disc_App_Timeout);
}
if (channel->pInterfaceStat->fax.Disc_Unspecified) {
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_LOG,
- "FAX Disc Unspec. = %lu", channel->pInterfaceStat->fax.Disc_Unspecified);
+ "FAX Disc Unspec. = %u", channel->pInterfaceStat->fax.Disc_Unspecified);
}
break;
}
diff --git a/drivers/isdn/hardware/eicon/maintidi.c b/drivers/isdn/hardware/eicon/maintidi.c
index 2ee789f95867..b2ed2939b4fa 100644
--- a/drivers/isdn/hardware/eicon/maintidi.c
+++ b/drivers/isdn/hardware/eicon/maintidi.c
@@ -30,7 +30,8 @@
#include "man_defs.h"
-extern void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
+extern __printf(3, 4)
+void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
#define MODEM_PARSE_ENTRIES 16 /* amount of variables of interest */
#define FAX_PARSE_ENTRIES 12 /* amount of variables of interest */
diff --git a/drivers/isdn/hardware/eicon/platform.h b/drivers/isdn/hardware/eicon/platform.h
index 62e2073c3690..efa574125d76 100644
--- a/drivers/isdn/hardware/eicon/platform.h
+++ b/drivers/isdn/hardware/eicon/platform.h
@@ -124,7 +124,7 @@
#define DIVAS_CONTAINING_RECORD(address, type, field) \
((type *)((char *)(address) - (char *)(&((type *)0)->field)))
-extern int sprintf(char *, const char *, ...);
+extern __printf(2, 3) int sprintf(char *, const char *, ...);
typedef void *LIST_ENTRY;
--
2.10.1
^ permalink raw reply related
* Re: [PATCH 03/17] batman-adv: Add network_coding and mcast sysfs files to README
From: Jiri Pirko @ 2016-10-29 13:51 UTC (permalink / raw)
To: Sven Eckelmann
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <2528373.sE2IHzpoFy@sven-edge>
Sat, Oct 29, 2016 at 01:46:59PM CEST, sven-KaDOiPu9UxWEi8DpZVb4nw@public.gmane.org wrote:
>On Samstag, 29. Oktober 2016 12:56:28 CEST Jiri Pirko wrote:
>[...]
>> >> I strongly believe it is a huge mistake to use sysfs for things like
>> >> this. This should be done via generic netlink api.
>> >
>> >This doesn't change the problem that it is already that way. This patch
>> >only adds the list of available files to the README.
>>
>> Sure. Just found out you did it like that. Therefore I commented. I
>> suggest to rework the api to use genl entirely.
>
>Fair enough, I have added it to the issue tracker [1].
>
>It seems there is no easy way to drop support for modifying batman-adv
>attributes of the interface or its ports via sysfs in the near
>future. But disallowing sysfs for new attributes might be a viable
>policy.
Cool. Thanks!
>
>Kind regards,
> Sven
>
>[1] https://www.open-mesh.org/issues/300
^ permalink raw reply
* Re: Let's do P4
From: Jiri Pirko @ 2016-10-29 13:58 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, jhs, roopa, john.fastabend, jakub.kicinski,
simon.horman, ast, daniel, prem, hannes, jbenc, tom, mattyk,
idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera
In-Reply-To: <20161029120932.GD1810@pox.localdomain>
Sat, Oct 29, 2016 at 02:09:32PM CEST, tgraf@suug.ch wrote:
>On 10/29/16 at 01:28pm, Jiri Pirko wrote:
>> Sat, Oct 29, 2016 at 01:15:48PM CEST, tgraf@suug.ch wrote:
>> >So given the SKIP_SW flag, the in-kernel compiler is optional anyway.
>> >Why even risk including a possibly incomplete compiler? Older kernels
>> >must be capable of running along newer hardware as long as eBPF can
>> >represent the software path. Having to upgrade to latest and greatest
>> >kernels is not an option for most people so they would simply have to
>> >fall back to SKIP_SW and do it in user space anyway.
>>
>> The thing is, if we needo to offload something, it needs to be
>> implemented in kernel first. Also, I believe that it is good to have
>> in-kernel p4 engine for testing and development purposes.
>
>You lost me now :-) In an earlier email you said:
>
>> It can be the other way around. The p4>ebpf compiler won't be complete
>> at the beginning so it is possible that HW could provide more features.
>> I don't think it is a problem. With SKIP_SW and SKIP_HW flags in TC,
>> the user can set different program to each. I think in real life, that
>> would be the most common case anyway.
>
>If you allow to SKIP_SW and set different programs each to address
>this, then how is this any different.
>
>I completely agree that kernel must be able to provide the same
>functionality as HW with optional additional capabilities on top so
>the HW can always bail out and punt to SW.
>
>[...]
>
>> >I'm not seeing how either of them is more or less variable. The main
>> >difference is whether to require configuring a single cls with both
>> >p4ast + bpf or two separate cls, one for each. I'd prefer the single
>> >cls approach simply because it is cleaner wither regard to offload
>> >directly off bpf vs off p4ast.
>>
>> That's the bundle that you asked me to forget earlier in this email? :)
>
>I thought you referred to the "store in same object file" as bundle.
>I don't really care about that. What I care about is a single way to
>configure this that works for both ASIC and non-ASIC hardware.
>
>> >My main point is to not include a IR to eBPF compiler in the kernel
>> >and let user space handle this instead.
>>
>> It we do it as you describe, we would be using 2 different APIs for
>> offloaded and non-offloaded path. I don't believe it is acceptable as
>> the offloaded features has to have kernel implementation. Therefore, I
>> believe that p4ast as a kernel API is the only possible option.
>
>Yes, the kernel has the SW implementation in eBPF. I thought that is
>what you propose as well. The only difference is whether to generate
>that eBPF in kernel or user space.
>
>Not sure I understand the multiple APIs point for offload vs
>non-offload. There is a single API: tc. Both models require the user
>to provide additional metadata to allow programming ASIC HW: p4ast
>IR or whatever we agree on.
If you do p4>ebpf in userspace, you have 2 apis:
1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
2) to setup hw p4 datapath, you push program.p4ast to kernel
Those are 2 apis. Both wrapped up by TC, but still 2 apis.
What I believe is correct is to have one api:
1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
2) to setup hw p4 datapath, you push program.p4ast to kernel
In case of 1), the program.p4ast will be either interpreted by new p4
interpreter, of translated to bpf and interpreted by that. But this
translation code is part of kernel.
^ permalink raw reply
* [PATCH net] qede: Fix out-of-bound fastpath memory access
From: Yuval Mintz @ 2016-10-29 14:04 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz
Driver allocates a shadow array for transmitted SKBs with X entries;
That means valid indices are {0,...,X - 1}. [X == 8191]
Problem is the driver also uses X as a mask for a
producer/consumer in order to choose the right entry in the
array which allows access to entry X which is out of bounds.
To fix this, simply allocate X + 1 entries in the shadow array.
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
Hi Dave,
Please consider applying this to `net'.
Thanks,
Yuval
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 444b271..7def29a 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2940,7 +2940,7 @@ static int qede_alloc_mem_txq(struct qede_dev *edev, struct qede_tx_queue *txq)
txq->num_tx_buffers = edev->q_num_tx_buffers;
/* Allocate the parallel driver ring for Tx buffers */
- size = sizeof(*txq->sw_tx_ring) * NUM_TX_BDS_MAX;
+ size = sizeof(*txq->sw_tx_ring) * TX_RING_SIZE;
txq->sw_tx_ring = kzalloc(size, GFP_KERNEL);
if (!txq->sw_tx_ring) {
DP_NOTICE(edev, "Tx buffers ring allocation failed\n");
@@ -2951,7 +2951,7 @@ static int qede_alloc_mem_txq(struct qede_dev *edev, struct qede_tx_queue *txq)
QED_CHAIN_USE_TO_CONSUME_PRODUCE,
QED_CHAIN_MODE_PBL,
QED_CHAIN_CNT_TYPE_U16,
- NUM_TX_BDS_MAX,
+ TX_RING_SIZE,
sizeof(*p_virt), &txq->tx_pbl);
if (rc)
goto err;
--
1.9.3
^ permalink raw reply related
* Re: Let's do P4
From: Jakub Kicinski @ 2016-10-29 14:49 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, tgraf, jhs, roopa, john.fastabend, simon.horman,
ast, daniel, prem, hannes, jbenc, tom, mattyk, idosch, eladr,
yotamg, nogahf, ogerlitz, linville, andy, f.fainelli, dsa,
vivien.didelot, andrew, ivecera, Maciej Żenczykowski
In-Reply-To: <20161029075328.GB1692@nanopsycho.orion>
On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
> Hi all.
>
> The network world is divided into 2 general types of hw:
> 1) network ASICs - network specific silicon, containing things like TCAM
> These ASICs are suitable to be programmed by P4.
> 2) network processors - basically a general purpose CPUs
> These processors are suitable to be programmed by eBPF.
>
> I believe that by now, the most people came to a conclusion that it is
> very difficult to handle both types by either P4 or eBPF. And since
> eBPF is part of the kernel, I would like to introduce P4 into kernel
> as well. Here's a plan:
>
> 1) Define P4 intermediate representation
> I cannot imagine loading P4 program (c-like syntax text file) into
> kernel as is. That means that as the first step, we need find some
> intermediate representation. I can imagine someting in a form of AST,
> call it "p4ast". I don't really know how to do this exactly though,
> it's just an idea.
>
> In the end there would be a userspace precompiler for this:
> $ makep4ast example.p4 example.ast
Maybe stating the obvious, but IMHO defining the IR is the hardest part.
eBPF *is* the IR, we can compile C, P4 or even JIT Lua to eBPF. The
AST/IR for switch pipelines should allow for similar flexibility.
Looser coupling would also protect us from changes in spec of the high
level language.
^ permalink raw reply
* Re: Let's do P4
From: Jakub Kicinski @ 2016-10-29 14:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: Thomas Graf, netdev, davem, jhs, roopa, john.fastabend,
simon.horman, ast, daniel, prem, hannes, jbenc, tom, mattyk,
idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera
In-Reply-To: <20161029135855.GH1692@nanopsycho.orion>
On Sat, 29 Oct 2016 15:58:55 +0200, Jiri Pirko wrote:
> Sat, Oct 29, 2016 at 02:09:32PM CEST, tgraf@suug.ch wrote:
> >On 10/29/16 at 01:28pm, Jiri Pirko wrote:
> >> Sat, Oct 29, 2016 at 01:15:48PM CEST, tgraf@suug.ch wrote:
> >> >So given the SKIP_SW flag, the in-kernel compiler is optional anyway.
> >> >Why even risk including a possibly incomplete compiler? Older kernels
> >> >must be capable of running along newer hardware as long as eBPF can
> >> >represent the software path. Having to upgrade to latest and greatest
> >> >kernels is not an option for most people so they would simply have to
> >> >fall back to SKIP_SW and do it in user space anyway.
> >>
> >> The thing is, if we needo to offload something, it needs to be
> >> implemented in kernel first. Also, I believe that it is good to have
> >> in-kernel p4 engine for testing and development purposes.
> >
> >You lost me now :-) In an earlier email you said:
> >
> >> It can be the other way around. The p4>ebpf compiler won't be complete
> >> at the beginning so it is possible that HW could provide more features.
> >> I don't think it is a problem. With SKIP_SW and SKIP_HW flags in TC,
> >> the user can set different program to each. I think in real life, that
> >> would be the most common case anyway.
> >
> >If you allow to SKIP_SW and set different programs each to address
> >this, then how is this any different.
> >
> >I completely agree that kernel must be able to provide the same
> >functionality as HW with optional additional capabilities on top so
> >the HW can always bail out and punt to SW.
> >
> >[...]
> >
> >> >I'm not seeing how either of them is more or less variable. The main
> >> >difference is whether to require configuring a single cls with both
> >> >p4ast + bpf or two separate cls, one for each. I'd prefer the single
> >> >cls approach simply because it is cleaner wither regard to offload
> >> >directly off bpf vs off p4ast.
> >>
> >> That's the bundle that you asked me to forget earlier in this email? :)
> >
> >I thought you referred to the "store in same object file" as bundle.
> >I don't really care about that. What I care about is a single way to
> >configure this that works for both ASIC and non-ASIC hardware.
> >
> >> >My main point is to not include a IR to eBPF compiler in the kernel
> >> >and let user space handle this instead.
> >>
> >> It we do it as you describe, we would be using 2 different APIs for
> >> offloaded and non-offloaded path. I don't believe it is acceptable as
> >> the offloaded features has to have kernel implementation. Therefore, I
> >> believe that p4ast as a kernel API is the only possible option.
> >
> >Yes, the kernel has the SW implementation in eBPF. I thought that is
> >what you propose as well. The only difference is whether to generate
> >that eBPF in kernel or user space.
> >
> >Not sure I understand the multiple APIs point for offload vs
> >non-offload. There is a single API: tc. Both models require the user
> >to provide additional metadata to allow programming ASIC HW: p4ast
> >IR or whatever we agree on.
>
> If you do p4>ebpf in userspace, you have 2 apis:
> 1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>
> Those are 2 apis. Both wrapped up by TC, but still 2 apis.
>
> What I believe is correct is to have one api:
> 1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>
> In case of 1), the program.p4ast will be either interpreted by new p4
> interpreter, of translated to bpf and interpreted by that. But this
> translation code is part of kernel.
Option 3) use a well structured subset of eBPF as user space ABI ;)
In all seriousness, user space already has to have some knowledge about
the underlaying hardware today with different vendors picking different
TC classifiers for offload. So I humbly agree that 2 APIs may be
acceptable here.
^ permalink raw reply
* Re: Let's do P4
From: Jiri Pirko @ 2016-10-29 14:55 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, tgraf, jhs, roopa, john.fastabend, simon.horman,
ast, daniel, prem, hannes, jbenc, tom, mattyk, idosch, eladr,
yotamg, nogahf, ogerlitz, linville, andy, f.fainelli, dsa,
vivien.didelot, andrew, ivecera, Maciej Żenczykowski
In-Reply-To: <20161029154903.25deb6db@jkicinski-Precision-T1700>
Sat, Oct 29, 2016 at 04:49:03PM CEST, kubakici@wp.pl wrote:
>On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
>> Hi all.
>>
>> The network world is divided into 2 general types of hw:
>> 1) network ASICs - network specific silicon, containing things like TCAM
>> These ASICs are suitable to be programmed by P4.
>> 2) network processors - basically a general purpose CPUs
>> These processors are suitable to be programmed by eBPF.
>>
>> I believe that by now, the most people came to a conclusion that it is
>> very difficult to handle both types by either P4 or eBPF. And since
>> eBPF is part of the kernel, I would like to introduce P4 into kernel
>> as well. Here's a plan:
>>
>> 1) Define P4 intermediate representation
>> I cannot imagine loading P4 program (c-like syntax text file) into
>> kernel as is. That means that as the first step, we need find some
>> intermediate representation. I can imagine someting in a form of AST,
>> call it "p4ast". I don't really know how to do this exactly though,
>> it's just an idea.
>>
>> In the end there would be a userspace precompiler for this:
>> $ makep4ast example.p4 example.ast
>
>Maybe stating the obvious, but IMHO defining the IR is the hardest part.
>eBPF *is* the IR, we can compile C, P4 or even JIT Lua to eBPF. The
>AST/IR for switch pipelines should allow for similar flexibility.
>Looser coupling would also protect us from changes in spec of the high
>level language.
Agreed. I agree with you point this would be nice to have it done in a
generic way. However, I'm not aware of any other language similar to p4.
^ permalink raw reply
* Re: Let's do P4
From: Jiri Pirko @ 2016-10-29 14:58 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Thomas Graf, netdev, davem, jhs, roopa, john.fastabend,
simon.horman, ast, daniel, prem, hannes, jbenc, tom, mattyk,
idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera
In-Reply-To: <20161029155421.02d81125@jkicinski-Precision-T1700>
Sat, Oct 29, 2016 at 04:54:21PM CEST, kubakici@wp.pl wrote:
>On Sat, 29 Oct 2016 15:58:55 +0200, Jiri Pirko wrote:
>> Sat, Oct 29, 2016 at 02:09:32PM CEST, tgraf@suug.ch wrote:
>> >On 10/29/16 at 01:28pm, Jiri Pirko wrote:
>> >> Sat, Oct 29, 2016 at 01:15:48PM CEST, tgraf@suug.ch wrote:
>> >> >So given the SKIP_SW flag, the in-kernel compiler is optional anyway.
>> >> >Why even risk including a possibly incomplete compiler? Older kernels
>> >> >must be capable of running along newer hardware as long as eBPF can
>> >> >represent the software path. Having to upgrade to latest and greatest
>> >> >kernels is not an option for most people so they would simply have to
>> >> >fall back to SKIP_SW and do it in user space anyway.
>> >>
>> >> The thing is, if we needo to offload something, it needs to be
>> >> implemented in kernel first. Also, I believe that it is good to have
>> >> in-kernel p4 engine for testing and development purposes.
>> >
>> >You lost me now :-) In an earlier email you said:
>> >
>> >> It can be the other way around. The p4>ebpf compiler won't be complete
>> >> at the beginning so it is possible that HW could provide more features.
>> >> I don't think it is a problem. With SKIP_SW and SKIP_HW flags in TC,
>> >> the user can set different program to each. I think in real life, that
>> >> would be the most common case anyway.
>> >
>> >If you allow to SKIP_SW and set different programs each to address
>> >this, then how is this any different.
>> >
>> >I completely agree that kernel must be able to provide the same
>> >functionality as HW with optional additional capabilities on top so
>> >the HW can always bail out and punt to SW.
>> >
>> >[...]
>> >
>> >> >I'm not seeing how either of them is more or less variable. The main
>> >> >difference is whether to require configuring a single cls with both
>> >> >p4ast + bpf or two separate cls, one for each. I'd prefer the single
>> >> >cls approach simply because it is cleaner wither regard to offload
>> >> >directly off bpf vs off p4ast.
>> >>
>> >> That's the bundle that you asked me to forget earlier in this email? :)
>> >
>> >I thought you referred to the "store in same object file" as bundle.
>> >I don't really care about that. What I care about is a single way to
>> >configure this that works for both ASIC and non-ASIC hardware.
>> >
>> >> >My main point is to not include a IR to eBPF compiler in the kernel
>> >> >and let user space handle this instead.
>> >>
>> >> It we do it as you describe, we would be using 2 different APIs for
>> >> offloaded and non-offloaded path. I don't believe it is acceptable as
>> >> the offloaded features has to have kernel implementation. Therefore, I
>> >> believe that p4ast as a kernel API is the only possible option.
>> >
>> >Yes, the kernel has the SW implementation in eBPF. I thought that is
>> >what you propose as well. The only difference is whether to generate
>> >that eBPF in kernel or user space.
>> >
>> >Not sure I understand the multiple APIs point for offload vs
>> >non-offload. There is a single API: tc. Both models require the user
>> >to provide additional metadata to allow programming ASIC HW: p4ast
>> >IR or whatever we agree on.
>>
>> If you do p4>ebpf in userspace, you have 2 apis:
>> 1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
>> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>>
>> Those are 2 apis. Both wrapped up by TC, but still 2 apis.
>>
>> What I believe is correct is to have one api:
>> 1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
>> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>>
>> In case of 1), the program.p4ast will be either interpreted by new p4
>> interpreter, of translated to bpf and interpreted by that. But this
>> translation code is part of kernel.
>
>Option 3) use a well structured subset of eBPF as user space ABI ;)
:( That would not be nice I believe. Also confusing and hard to
maintain. Plus we would have to do 2 translations, in between
incompatible paradigms.
>
>In all seriousness, user space already has to have some knowledge about
>the underlaying hardware today with different vendors picking different
>TC classifiers for offload. So I humbly agree that 2 APIs may be
>acceptable here.
^ permalink raw reply
* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: Lorenzo Colitti @ 2016-10-29 15:34 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Mack, Pablo Neira Ayuso, htejun, Daniel Borkmann, ast,
David Miller, kafai, Florian Westphal, harald,
netdev@vger.kernel.org, Sargun Dhillon, cgroups
In-Reply-To: <20161029062442.GA61550@ast-mbp.thefacebook.com>
On Sat, Oct 29, 2016 at 3:24 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> it could be solved by swapping the order of cgroup_bpf_run_filter()
> and NF_INET_POST_ROUTING in patch 5. It was proposed some time back, but
> the current patch, I think, is more symmetrical.
> cgroup+bpf runs after nf hook on rx and runs before it on tx.
> imo it's more consistent.
I guess what I was trying to say was: what does doing this filtering
in ip_output give you over running this from the netfilter hooks?
Doing this filtering in netfilter is much more general because there
can be complex rules both before and after the filtering is applied. I
hadn't thought of the scalability issue you note below though.
For accounting you probably want to run after the hooks, both for
ingress and for egress, because the hooks can do all sorts of stuff
like drop packets, change packet sizes, reroute them to different
interfaces, etc. Do you see use cases where you want to run before the
hooks?
> Regardless of this choice... are you going to backport cgroupv2 to
> android? Because this set is v2 only.
Certainly anything that can't easily be backported to, say,
android-4.4 is not really feasible in the short term. I don't think we
use network cgroups at all, so if v2 network cgroups can coexist with
v1 cgroups of other types (which what little I've read seems to
indicate) then that should be possible.
> yes. that's certainly doable, but sooner or later such approach will hit
> scalability issue when number of cgroups is large. Same issue we saw
> with cls_bpf and bpf_skb_under_cgroup(). Hence this patch set was needed
> that is centered around cgroups instead of hooks. Note, unlike, tc and nf
> there is no way to attach to a hook. The bpf program is attached to a cgroup.
> It's an important distinction vs everything that currently exists in the stack.
Ah, I see. Out of curiosity, what was the first scaling limitation you
hit? eBPF program length? eBPF map size?
^ permalink raw reply
* Re: [PATCH net 00/12] Mellanox 100G mlx5 fixes 2016-10-25
From: David Miller @ 2016-10-29 15:56 UTC (permalink / raw)
To: saeedm; +Cc: netdev, ogerlitz, leonro
In-Reply-To: <1477409795-28807-1-git-send-email-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 25 Oct 2016 18:36:23 +0300
> This series contains some bug fixes for the mlx5 core and mlx5e
> driver.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net] sctp: validate chunk len before actually using it
From: David Miller @ 2016-10-29 15:57 UTC (permalink / raw)
To: marcelo.leitner
Cc: netdev, linux-sctp, nhorman, vyasevich, syzkaller, kcc, glider,
edumazet, dvyukov, andreyknvl
In-Reply-To: <06d1d4f21f959784823d7c7e31c3ea7e7360cb99.1477410525.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Tue, 25 Oct 2016 14:27:39 -0200
> Andrey Konovalov reported that KASAN detected that SCTP was using a slab
> beyond the boundaries. It was caused because when handling out of the
> blue packets in function sctp_sf_ootb() it was checking the chunk len
> only after already processing the first chunk, validating only for the
> 2nd and subsequent ones.
>
> The fix is to just move the check upwards so it's also validated for the
> 1st chunk.
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>
> Hi. Please consider this to -stable too. Thanks
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH v2 net-next] virtio-net: Update the mtu code to match virtio spec
From: David Miller @ 2016-10-29 16:01 UTC (permalink / raw)
To: aconole; +Cc: jarod, netdev, mst, linux-kernel, virtualization
In-Reply-To: <1477426332-31093-1-git-send-email-aconole@redhat.com>
From: Aaron Conole <aconole@redhat.com>
Date: Tue, 25 Oct 2016 16:12:12 -0400
> The virtio committee recently ratified a change, VIRTIO-152, which
> defines the mtu field to be 'max' MTU, not simply desired MTU.
>
> This commit brings the virtio-net device in compliance with VIRTIO-152.
>
> Additionally, drop the max_mtu branch - it cannot be taken since the u16
> returned by virtio_cread16 will never exceed the initial value of
> max_mtu.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Acked-by: "Michael S. Tsirkin" <mst@redhat.com>
> Acked-by: Jarod Wilson <jarod@redhat.com>
> ---
> Nothing code-wise has changed, but I've included the ACKs and fixed up the
> subject line.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] inet: Fix missing return value in inet6_hash
From: David Miller @ 2016-10-29 16:02 UTC (permalink / raw)
To: kraigatgoog; +Cc: soheil, netdev
In-Reply-To: <1477433329-165391-1-git-send-email-kraigatgoog@gmail.com>
From: Craig Gallek <kraigatgoog@gmail.com>
Date: Tue, 25 Oct 2016 18:08:49 -0400
> From: Craig Gallek <kraig@google.com>
>
> As part of a series to implement faster SO_REUSEPORT lookups,
> commit 086c653f5862 ("sock: struct proto hash function may error")
> added return values to protocol hash functions and
> commit 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
> implemented a new hash function for IPv6. However, the latter does
> not respect the former's convention.
>
> This properly propagates the hash errors in the IPv6 case.
>
> Fixes: 496611d7b5ea ("inet: create IPv6-equivalent inet_hash function")
> Reported-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Craig Gallek <kraig@google.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: Let's do P4
From: John Fastabend @ 2016-10-29 16:46 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Pirko
Cc: netdev, davem, tgraf, jhs, roopa, simon.horman, ast, daniel, prem,
hannes, jbenc, tom, mattyk, idosch, eladr, yotamg, nogahf,
ogerlitz, linville, andy, f.fainelli, dsa, vivien.didelot, andrew,
ivecera, Maciej Żenczykowski
In-Reply-To: <20161029154903.25deb6db@jkicinski-Precision-T1700>
On 16-10-29 07:49 AM, Jakub Kicinski wrote:
> On Sat, 29 Oct 2016 09:53:28 +0200, Jiri Pirko wrote:
>> Hi all.
>>
>> The network world is divided into 2 general types of hw:
>> 1) network ASICs - network specific silicon, containing things like TCAM
>> These ASICs are suitable to be programmed by P4.
>> 2) network processors - basically a general purpose CPUs
>> These processors are suitable to be programmed by eBPF.
>>
>> I believe that by now, the most people came to a conclusion that it is
>> very difficult to handle both types by either P4 or eBPF. And since
>> eBPF is part of the kernel, I would like to introduce P4 into kernel
>> as well. Here's a plan:
>>
>> 1) Define P4 intermediate representation
>> I cannot imagine loading P4 program (c-like syntax text file) into
>> kernel as is. That means that as the first step, we need find some
>> intermediate representation. I can imagine someting in a form of AST,
>> call it "p4ast". I don't really know how to do this exactly though,
>> it's just an idea.
>>
>> In the end there would be a userspace precompiler for this:
>> $ makep4ast example.p4 example.ast
>
> Maybe stating the obvious, but IMHO defining the IR is the hardest part.
> eBPF *is* the IR, we can compile C, P4 or even JIT Lua to eBPF. The
> AST/IR for switch pipelines should allow for similar flexibility.
> Looser coupling would also protect us from changes in spec of the high
> level language.
>
Jumping in the middle here. You managed to get an entire thread going
before I even woke up :)
The problem with eBPF as an IR is that in the universe of eBPF IR
programs the subset that can be offloaded onto a standard ASIC based
hardware (non NPU/FPGA/etc) is so small to be almost meaningless IMO.
I tried this for awhile and the result is users have to write very
targeted eBPF that they "know" will be pattern matched and pushed into
an ASIC. It can work but its very fragile. When I did this I ended up
with an eBPF generator for deviceX and an eBPF generator for deviceY
each with a very specific pattern matching engine in the driver to
xlate ebpf-deviceX into its asic. Existing ASICs for example usually
support only one pipeline, only one parser (or require moving mountains
to change the parse via ucode), only one set of tables, and only one
deparser/serailizer at the end to build the new packet. Next-gen pieces
may have some flexibility on the parser side.
There is an interesting resource allocation problem we have that could
be solved by p4 or devlink where in we want to pre-allocate slices of
the TCAM for certain match types. I was planning on writing devlink code
for this because its primarily done at initialization once.
I will note one nice thing about using eBPF however is that you have an
easy software emulation path via ebpf engine in kernel.
... And merging threads here with Jiri's email ...
> If you do p4>ebpf in userspace, you have 2 apis:
> 1) to setup sw (in-kernel) p4 datapath, you push bpf.o to kernel
> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>
> Those are 2 apis. Both wrapped up by TC, but still 2 apis.
>
> What I believe is correct is to have one api:
> 1) to setup sw (in-kernel) p4 datapath, you push program.p4ast to kernel
> 2) to setup hw p4 datapath, you push program.p4ast to kernel
>
Couple comments around this, first adding yet another IR in the kernel
and another JIT engine to map that IR on to eBPF or hardware vendor X
doesn't get me excited. Its really much easier to write these as backend
objects in LLVM. Not saying it can't be done just saying it is easier
in LLVM. Also we already have the LLVM code for P4 to LLVM-IR to eBPF.
In the end this would be a reasonably complex bit of code in
the kernel only for hardware offload. I have doubts that folks would
ever use it for software only cases. I'm happy to admit I'm wrong here
though.
So yes using llvm backends creates two paths a hardware mgmt and sw
path but in the hardware + software case typical on the edge the
orchestration and management planes have started to manage the hardware
and software as two blocks of logic for performance SLA logic. Even on
the edge it seems in most cases folks are selling SR-IOV ports and
can't fall back to software and charge for the port. But this is just
one use case I suspect others where it does make sense.
> In case of 1), the program.p4ast will be either interpreted by new p4
> interpreter, of translated to bpf and interpreted by that. But this
> translation code is part of kernel.
Finally a couple historic bits. The Flow-API proposed in Ottawa was
mechanically generated from an original P4 draft. At the time I was
working fairly closely with both the hardware and compiler folks. If
there is interest we could use that as a base IR for hardware. It has
a simple mapping to/from the original P4 spec. The newer P4 specs are
significantly more complex by the way.
We also have an emulated path also auto-generated from compiler tools
that creates eBPF code from the IR so this would give you the software
fall-back.
It is something we could spin up an RFC in a few weeks if there is some
agreement here. I'll be traveling though for a week or two but could
get something out in November.
Thanks,
John
^ permalink raw reply
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Andrey Konovalov @ 2016-10-29 17:06 UTC (permalink / raw)
To: Cong Wang
Cc: Gerrit Renker, David S. Miller, dccp, netdev, LKML, Dmitry Vyukov,
Eric Dumazet, Alexander Potapenko, Kostya Serebryany, syzkaller
In-Reply-To: <CAM_iQpX_xw35MUyd9aON5U=HmUoRafeoOF+_vSTHGYnrXd-CHg@mail.gmail.com>
Hi Cong,
Tested with your patch, still getting a warning, though it's a little different:
------------[ cut here ]------------
WARNING: CPU: 1 PID: 3876 at kernel/sched/core.c:7724
__might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
do not call blocking ops when !TASK_RUNNING; state=1 set at
[<ffffffff811f5a5c>] prepare_to_wait+0xbc/0x210
kernel/sched/wait.c:178
Modules linked in:
CPU: 1 PID: 3876 Comm: a.out Not tainted 4.9.0-rc2+ #325
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
ffff88006c2d7770 ffffffff81b46914 ffff88006c2d77e8 0000000000000000
ffffffff84052960 0000000000000000 ffff88006c2d77b8 ffffffff81111237
0000000041b58ab3 ffffffff00001e2c ffffed000d85aef9 ffffffff84052960
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff81b46914>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
[<ffffffff81111237>] __warn+0x1a7/0x1f0 kernel/panic.c:550
[<ffffffff8111132c>] warn_slowpath_fmt+0xac/0xd0 kernel/panic.c:565
[<ffffffff811922fc>] __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
[< inline >] slab_pre_alloc_hook mm/slab.h:393
[< inline >] slab_alloc_node mm/slub.c:2634
[< inline >] slab_alloc mm/slub.c:2716
[<ffffffff815054fb>] kmem_cache_alloc_trace+0x1bb/0x270 mm/slub.c:2733
[< inline >] kmalloc ./include/linux/slab.h:490
[<ffffffff83889e62>] dccp_feat_entry_new+0x182/0x2a0 net/dccp/feat.c:468
[<ffffffff8388a1ea>] dccp_feat_push_confirm+0x3a/0x270 net/dccp/feat.c:516
[< inline >] dccp_feat_change_recv net/dccp/feat.c:1160
[<ffffffff8388d587>] dccp_feat_parse_options+0xb37/0x13d0 net/dccp/feat.c:1412
[<ffffffff83894f61>] dccp_parse_options+0x721/0x1010 net/dccp/options.c:128
[<ffffffff838912e0>] dccp_rcv_state_process+0x200/0x15b0 net/dccp/input.c:644
[<ffffffff838b8af4>] dccp_v4_do_rcv+0xf4/0x1a0 net/dccp/ipv4.c:681
[< inline >] sk_backlog_rcv ./include/net/sock.h:872
[<ffffffff82b7ceb6>] __release_sock+0x126/0x3a0 net/core/sock.c:2044
[<ffffffff82b7d189>] release_sock+0x59/0x1c0 net/core/sock.c:2502
[< inline >] inet_wait_for_connect net/ipv4/af_inet.c:547
[<ffffffff8316b2a2>] __inet_stream_connect+0x5d2/0xbb0 net/ipv4/af_inet.c:617
[<ffffffff8316b8d5>] inet_stream_connect+0x55/0xa0 net/ipv4/af_inet.c:656
[<ffffffff82b705e4>] SYSC_connect+0x244/0x2f0 net/socket.c:1533
[<ffffffff82b72dd4>] SyS_connect+0x24/0x30 net/socket.c:1514
[<ffffffff83fbf781>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
---[ end trace c7e036cf4dc54077 ]---
Thanks!
On Sat, Oct 29, 2016 at 8:10 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Oct 28, 2016 at 5:40 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> Hi,
>>
>> I've got the following error report while running the syzkaller fuzzer:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 4608 at kernel/sched/core.c:7724
>> __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
>> do not call blocking ops when !TASK_RUNNING; state=1 set at
>> [<ffffffff811f5a5c>] prepare_to_wait+0xbc/0x210
>> kernel/sched/wait.c:178
>> Modules linked in:
>> CPU: 0 PID: 4608 Comm: syz-executor Not tainted 4.9.0-rc2+ #320
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> ffff88006625f7a0 ffffffff81b46914 ffff88006625f818 0000000000000000
>> ffffffff84052960 0000000000000000 ffff88006625f7e8 ffffffff81111237
>> ffff88006aceac00 ffffffff00001e2c ffffed000cc4beff ffffffff84052960
>> Call Trace:
>> [< inline >] __dump_stack lib/dump_stack.c:15
>> [<ffffffff81b46914>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
>> [<ffffffff81111237>] __warn+0x1a7/0x1f0 kernel/panic.c:550
>> [<ffffffff8111132c>] warn_slowpath_fmt+0xac/0xd0 kernel/panic.c:565
>> [<ffffffff811922fc>] __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
>> [< inline >] slab_pre_alloc_hook mm/slab.h:393
>> [< inline >] slab_alloc_node mm/slub.c:2634
>> [< inline >] slab_alloc mm/slub.c:2716
>> [<ffffffff81508da0>] __kmalloc_track_caller+0x150/0x2a0 mm/slub.c:4240
>> [<ffffffff8146be14>] kmemdup+0x24/0x50 mm/util.c:113
>> [<ffffffff8388b2cf>] dccp_feat_clone_sp_val.part.5+0x4f/0xe0
>> net/dccp/feat.c:374
>> [< inline >] dccp_feat_clone_sp_val net/dccp/feat.c:1141
>> [< inline >] dccp_feat_change_recv net/dccp/feat.c:1141
>> [<ffffffff8388d491>] dccp_feat_parse_options+0xaa1/0x13d0 net/dccp/feat.c:1411
>> [<ffffffff83894f01>] dccp_parse_options+0x721/0x1010 net/dccp/options.c:128
>> [<ffffffff83891280>] dccp_rcv_state_process+0x200/0x15b0 net/dccp/input.c:644
>> [<ffffffff838b8a94>] dccp_v4_do_rcv+0xf4/0x1a0 net/dccp/ipv4.c:681
>> [< inline >] sk_backlog_rcv ./include/net/sock.h:872
>> [<ffffffff82b7ceb6>] __release_sock+0x126/0x3a0 net/core/sock.c:2044
>> [<ffffffff82b7d189>] release_sock+0x59/0x1c0 net/core/sock.c:2502
>> [< inline >] inet_wait_for_connect net/ipv4/af_inet.c:547
>> [<ffffffff8316b2a2>] __inet_stream_connect+0x5d2/0xbb0 net/ipv4/af_inet.c:617
>> [<ffffffff8316b8d5>] inet_stream_connect+0x55/0xa0 net/ipv4/af_inet.c:656
>> [<ffffffff82b705e4>] SYSC_connect+0x244/0x2f0 net/socket.c:1533
>> [<ffffffff82b72dd4>] SyS_connect+0x24/0x30 net/socket.c:1514
>> [<ffffffff83fbf701>] entry_SYSCALL_64_fastpath+0x1f/0xc2
>> arch/x86/entry/entry_64.S:209
>
> Should be fixed the attached patch. I will verify it with your
> reproducer tomorrow.
>
> Thanks!
^ permalink raw reply
* Re: [PATCH net 2/3] sctp: return back transport in __sctp_rcv_init_lookup
From: Xin Long @ 2016-10-29 17:29 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: network dev, linux-sctp, davem, Vlad Yasevich, Daniel Borkmann
In-Reply-To: <20161028213943.GA14175@localhost.localdomain>
On Sat, Oct 29, 2016 at 5:39 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Fri, Oct 28, 2016 at 05:42:21PM -0200, Marcelo Ricardo Leitner wrote:
>> On Fri, Oct 28, 2016 at 06:10:53PM +0800, Xin Long wrote:
>> > Prior to this patch, it used a local variable to save the transport that is
>> > looked up by __sctp_lookup_association(), and didn't return it back. But in
>> > sctp_rcv, it is used to initialize chunk->transport. So when hitting this
>> > code, it was initializing chunk->transport with some random stack value
>> > instead.
here should be:
So when hitting this, even if it found the transport, it was still initializing
chunk->transport with null instead.
>> >
>> > This patch is to return the transport back through transport pointer
>> > that is from __sctp_rcv_lookup_harder().
>> >
>> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>
>> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>>
>> transport pointer in sctp_rcv() is initialized to null and there are
>> checks for it after this path, so this shouldn't be exploitable, just
>> malfunction.
>
> This actually sort of contradicts the changelog.
>
> Xin, did I miss something here? Seems we need to update the changelog if
> not.
>
You're right, thanks, will repost.
^ permalink raw reply
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Eric Dumazet @ 2016-10-29 17:43 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Cong Wang, Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Eric Dumazet
In-Reply-To: <CAAeHK+y9ENvGzJEQtRy_5DEzCfhGEYXR5CPpgwXy_FoOtFUf3Q@mail.gmail.com>
On Sat, 2016-10-29 at 19:06 +0200, Andrey Konovalov wrote:
> Hi Cong,
>
> Tested with your patch, still getting a warning, though it's a little different:
>
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 3876 at kernel/sched/core.c:7724
> __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
> do not call blocking ops when !TASK_RUNNING; state=1 set at
> [<ffffffff811f5a5c>] prepare_to_wait+0xbc/0x210
> kernel/sched/wait.c:178
> Modules linked in:
This looks like the following patch is needed, can you test it ?
Thanks !
diff --git a/net/dccp/output.c b/net/dccp/output.c
index b66c84db0766..74d8583a0d52 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -228,6 +228,7 @@ static int dccp_wait_for_ccid(struct sock *sk, unsigned long delay)
remaining = schedule_timeout(delay);
+ sched_annotate_sleep();
lock_sock(sk);
sk->sk_write_pending--;
finish_wait(sk_sleep(sk), &wait);
^ permalink raw reply related
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Andrey Konovalov @ 2016-10-29 17:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: Cong Wang, Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Eric Dumazet
In-Reply-To: <1477762981.7065.272.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
Tested with both patches applied, still seeing the warning.
Thanks!
On Sat, Oct 29, 2016 at 7:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2016-10-29 at 19:06 +0200, Andrey Konovalov wrote:
>> Hi Cong,
>>
>> Tested with your patch, still getting a warning, though it's a little different:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 1 PID: 3876 at kernel/sched/core.c:7724
>> __might_sleep+0x14c/0x1a0 kernel/sched/core.c:7719
>> do not call blocking ops when !TASK_RUNNING; state=1 set at
>> [<ffffffff811f5a5c>] prepare_to_wait+0xbc/0x210
>> kernel/sched/wait.c:178
>> Modules linked in:
>
> This looks like the following patch is needed, can you test it ?
> Thanks !
>
> diff --git a/net/dccp/output.c b/net/dccp/output.c
> index b66c84db0766..74d8583a0d52 100644
> --- a/net/dccp/output.c
> +++ b/net/dccp/output.c
> @@ -228,6 +228,7 @@ static int dccp_wait_for_ccid(struct sock *sk, unsigned long delay)
>
> remaining = schedule_timeout(delay);
>
> + sched_annotate_sleep();
> lock_sock(sk);
> sk->sk_write_pending--;
> finish_wait(sk_sleep(sk), &wait);
>
>
>
^ permalink raw reply
* [PATCH net] net: mangle zero checksum in skb_checksum_help()
From: Eric Dumazet @ 2016-10-29 18:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Maciej Żenczykowski, Willem de Bruijn
From: Eric Dumazet <edumazet@google.com>
Sending zero checksum is ok for TCP, but not for UDP.
UDPv6 receiver should by default drop a frame with a 0 checksum,
and UDPv4 would not verify the checksum and might accept a corrupted
packet.
Simply replace such checksum by 0xffff, regardless of transport.
This error was caught on SIT tunnels, but seems generic.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index dbc871306910..899a0f00e721 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2484,7 +2484,7 @@ int skb_checksum_help(struct sk_buff *skb)
goto out;
}
- *(__sum16 *)(skb->data + offset) = csum_fold(csum);
+ *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
out_set_summed:
skb->ip_summed = CHECKSUM_NONE;
out:
^ permalink raw reply related
* Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep
From: Eric Dumazet @ 2016-10-29 18:05 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Cong Wang, Gerrit Renker, David S. Miller, dccp, netdev, LKML,
Dmitry Vyukov, Eric Dumazet
In-Reply-To: <CAAeHK+x6+CU+1KJNOpnoyyZoqH7_dcrWFVGU1KrJYVwJSzdxPg@mail.gmail.com>
On Sat, 2016-10-29 at 19:59 +0200, Andrey Konovalov wrote:
> Hi Eric,
>
> Tested with both patches applied, still seeing the warning.
>
> Thanks!
Arg, sorry, this was at the wrong place.
Thanks for testing !
diff --git a/net/dccp/output.c b/net/dccp/output.c
index b66c84db0766..2548edff86ff 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -224,6 +224,11 @@ static int dccp_wait_for_ccid(struct sock *sk, unsigned long delay)
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sk->sk_write_pending++;
+
+ /* release_sock()/lock_sock() will process socket backlog
+ * from process context. Be prepared to sleep !
+ */
+ sched_annotate_sleep();
release_sock(sk);
remaining = schedule_timeout(delay);
^ permalink raw reply related
* Re: [PATCH net] net: mangle zero checksum in skb_checksum_help()
From: Maciej Żenczykowski @ 2016-10-29 18:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1477764156.7065.282.camel@edumazet-glaptop3.roam.corp.google.com>
Acked-by: Maciej Żenczykowski <maze@google.com>
^ permalink raw reply
* Re: [PATCH net-next V3 0/9] liquidio CN23XX VF support
From: David Miller @ 2016-10-29 18:31 UTC (permalink / raw)
To: rvatsavayi; +Cc: netdev
In-Reply-To: <1477443430-27170-1-git-send-email-rvatsavayi@caviumnetworks.com>
From: Raghu Vatsavayi <rvatsavayi@caviumnetworks.com>
Date: Tue, 25 Oct 2016 17:57:01 -0700
> 2) As recommended by you removed custom module parameter max_vfs.
I feel the same way about num_queues_per_{p,v}f.
What's really strange is that there is a reference to num_queues_per_pf
in one of the kernel log messages already.
^ permalink raw reply
* Re: [PATCH net] bpf: fix samples to add fake KBUILD_MODNAME
From: David Miller @ 2016-10-29 18:46 UTC (permalink / raw)
To: daniel; +Cc: alexei.starovoitov, netdev
In-Reply-To: <1fdac65566ed6e0cff486507c5019f45807d1e89.1477434641.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 26 Oct 2016 00:37:53 +0200
> Some of the sample files are causing issues when they are loaded with tc
> and cls_bpf, meaning tc bails out while trying to parse the resulting ELF
> file as program/map/etc sections are not present, which can be easily
> spotted with readelf(1).
>
> Currently, BPF samples are including some of the kernel headers and mid
> term we should change them to refrain from this, really. When dynamic
> debugging is enabled, we bail out due to undeclared KBUILD_MODNAME, which
> is easily overlooked in the build as clang spills this along with other
> noisy warnings from various header includes, and llc still generates an
> ELF file with mentioned characteristics. For just playing around with BPF
> examples, this can be a bit of a hurdle to take.
>
> Just add a fake KBUILD_MODNAME as a band-aid to fix the issue, same is
> done in xdp*_kern samples already.
>
> Fixes: 65d472fb007d ("samples/bpf: add 'pointer to packet' tests")
> Fixes: 6afb1e28b859 ("samples/bpf: Add tunnel set/get tests.")
> Fixes: a3f74617340b ("cgroup: bpf: Add an example to do cgroup checking in BPF")
> Reported-by: Chandrasekar Kannan <ckannan@console.to>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied.
^ permalink raw reply
* Re: [PATCH] ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()
From: David Miller @ 2016-10-29 18:50 UTC (permalink / raw)
To: elicooper; +Cc: netdev, tom
In-Reply-To: <20161026021109.19277-1-elicooper@gmx.com>
From: Eli Cooper <elicooper@gmx.com>
Date: Wed, 26 Oct 2016 10:11:09 +0800
> This patch updates skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit() when an
> IPv6 header is installed to a socket buffer.
>
> This is not a cosmetic change. Without updating this value, GSO packets
> transmitted through an ipip6 tunnel have the protocol of ETH_P_IP and
> skb_mac_gso_segment() will attempt to call gso_segment() for IPv4,
> which results in the packets being dropped.
>
> Fixes: b8921ca83eed ("ip4ip6: Support for GSO/GRO")
> Signed-off-by: Eli Cooper <elicooper@gmx.com>
Applied and queued up for -stable, thank you.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox